Page MenuHomePhabricator
Differential D4773

[tunnelbroker] Removed unused include directives
ClosedPublic
Actions

Authored by atul on Aug 8 2022, 12:47 PM.
Tags
None
Referenced Files
F3540331: D4773.id15448.diff
Thu, Dec 26, 4:45 AM
Unknown Object (File)
Sun, Dec 15, 10:24 PM
Unknown Object (File)
Sun, Dec 15, 10:24 PM
Unknown Object (File)
Sun, Dec 15, 10:24 PM
Unknown Object (File)
Sun, Dec 15, 10:24 PM
Unknown Object (File)
Sun, Dec 15, 10:24 PM
Unknown Object (File)
Thu, Dec 5, 10:20 PM
Unknown Object (File)
Nov 8 2024, 8:58 PM
View All Files
Subscribers
• adrian
ashoat

Details

Reviewers
• jon
• abosh
varun
tomek
• max
• karol
Commits
rCOMM8d121152a74e: [tunnelbroker] Removed unused include directives
Summary

Just a minor thing I noticed while testing some of the nix tunnelbroker diffs.

Test Plan

NA, CI?

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

atul created this revision.Aug 8 2022, 12:47 PM
Herald added subscribers: • adrian, ashoat. · View Herald TranscriptAug 8 2022, 12:47 PM
Harbormaster completed remote builds in B11230: Diff 15448.Aug 8 2022, 12:59 PM
atul requested review of this revision.Aug 8 2022, 12:59 PM
• abosh requested changes to this revision.Aug 8 2022, 1:03 PM

Don't think the test plan should be NA here, even if it may seem like a trivial change. How were you able to determine these #include directives were unused in the first place? Was it something your IDE picked up on...or?

This revision now requires changes to proceed.Aug 8 2022, 1:03 PM
atul requested review of this revision.Aug 8 2022, 1:07 PM

Was it something your IDE picked up on...or?

Ya

• abosh accepted this revision.Aug 8 2022, 1:08 PM
In D4773#137756, @atul wrote:

Was it something your IDE picked up on...or?

Ya

Alrighty then, sounds good to me

This revision is now accepted and ready to land.Aug 8 2022, 1:08 PM
atul updated this revision to Diff 15449.Aug 8 2022, 1:19 PM

rebase onto master before landing

Harbormaster failed remote builds in B11231: Diff 15449!Aug 8 2022, 1:19 PM
varun accepted this revision.Aug 8 2022, 1:22 PM

Some builds failed but they seem unrelated

atul added a comment.Aug 8 2022, 1:27 PM
In D4773#137774, @varun wrote:

Some builds failed but they seem unrelated

Yeah that was my bad, was messing with a buildkite agent

Harbormaster completed remote builds in B11231: Diff 15449.Aug 8 2022, 1:34 PM
• jon added inline comments.Aug 8 2022, 1:39 PM
services/tunnelbroker/src/server.cpp
3 ↗(On Diff #15449)

hold off 2s, I think I'm changing this as well

atul added inline comments.Aug 8 2022, 1:41 PM
services/tunnelbroker/src/server.cpp
3 ↗(On Diff #15449)

thanks for heads up, I'll hold off on landing until you've landed your stack

• jon accepted this revision.Aug 8 2022, 1:47 PM

Nevermind, I don't think any of my changes overlapped

• jon added a comment.Aug 8 2022, 1:47 PM

I would still rebase to ensure that the changes applied cleanly

atul updated this revision to Diff 15459.Aug 8 2022, 1:51 PM

rebase after jon stack

Harbormaster completed remote builds in B11232: Diff 15459.Aug 8 2022, 2:05 PM
Closed by commit rCOMM8d121152a74e: [tunnelbroker] Removed unused include directives (authored by atul). · Explain WhyAug 8 2022, 2:06 PM
This revision was automatically updated to reflect the committed changes.
atul added a commit: rCOMM8d121152a74e: [tunnelbroker] Removed unused include directives.

Revision Contents
Changeset List

PathSize
services/
tunnelbroker/
src/
Amqp/
1 line
Tools/
3 lines
1 line
1 line

Diff 15460

View Options

services/tunnelbroker/src/Amqp/AmqpManager.cpp

Loading...
View Options

services/tunnelbroker/src/Tools/CryptoTools.cpp

Loading...
View Options

services/tunnelbroker/src/Tools/Tools.cpp

Loading...
View Options

services/tunnelbroker/src/server.cpp

Loading...