Page MenuHomePhabricator

[services] Tunnelbroker - Removing of deprecated C++ files
ClosedPublic

Authored by max on Oct 20 2022, 7:36 AM.
Tags
None
Referenced Files
F3361245: D5431.id18102.diff
Sun, Nov 24, 3:55 PM
F3361221: D5431.id17714.diff
Sun, Nov 24, 3:49 PM
F3361165: D5431.id18112.diff
Sun, Nov 24, 3:34 PM
F3361147: D5431.id17839.diff
Sun, Nov 24, 3:30 PM
F3360539: D5431.diff
Sun, Nov 24, 1:18 PM
Unknown Object (File)
Thu, Nov 21, 10:16 PM
Unknown Object (File)
Sat, Nov 16, 8:11 AM
Unknown Object (File)
Fri, Nov 15, 1:48 AM

Details

Summary

This diff is a part of the flat diffs stack to transform the current Tunnelbroker project into a Rust app by calling the current C++ codebase from Rust. The related transition task is ENG-1072.

This diff removes the deprecated and unnecessary files from the project structure because of switching to the Rust application.

Removed files description:

  • server.cpp - Entry point for C++ application, which will be replaced by the main.rs in Rust.
  • Service/TunnelbrokerServiceImpl.cpp, Service/TunnelbrokerServiceImpl.h - C++ gRPC server handlers implementation, will be replaced by the server module in Rust.

The project directory structure task and discussion: ENG-2083

CI Notice:
The CI will fail on this and further diffs until the D5461 where the changes to CI build commands are made.
This stack will be landed all in one to prevent CI from failing on diffs out of this stack.

Test Plan

1. CI Testing:
These diff changes are included as a part of the stack in the later D5461 diff, where the changes to CI build commands are made. The CI succeded on the D5461 which reflects that this changes are passing the build.

2. Manually testing:
These diff changes can be tested in the middle of the stack at D5436. To test it patch to D5436 using arc patch D5436, run nix develop. Go to tunnelbroker service directory services/tunnelbroker and run the Cargo building process cargo build or cargo run.
The Rust application will be successfully built.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

max held this revision as a draft.
max edited the test plan for this revision. (Show Details)
max added reviewers: varun, marcin. max added 1 blocking reviewer(s): jon.
max published this revision for review.Oct 20 2022, 8:24 AM

Updating stack dependencies.

max edited the test plan for this revision. (Show Details)
This revision is now accepted and ready to land.Oct 25 2022, 5:39 PM
This revision now requires review to proceed.Oct 26 2022, 2:23 AM

This diff removes a lot of code. In the previous diff we added only a part of it. I assume in the next diffs most of this code will be reintroduced, but that makes it hard to verify if the code remains correct. It would be better if parts of the code were moved in a diff instead of removing in one and adding in another.

This revision is now accepted and ready to land.Oct 27 2022, 4:30 AM
In D5431#162401, @tomek wrote:

This diff removes a lot of code. In the previous diff we added only a part of it. I assume in the next diffs most of this code will be reintroduced, but that makes it hard to verify if the code remains correct. It would be better if parts of the code were moved in a diff instead of removing in one and adding in another.

It makes sense if the API remains unchanged, but in our case, the API is changed and it seems it doesn't make sense to refer to the old code with the old API.

Rebasing on parent changes.

This revision was landed with ongoing or failed builds.Nov 7 2022, 5:56 AM
This revision was automatically updated to reflect the committed changes.