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
F2151479: D4773.id15460.diff
Sun, Jun 30, 12:49 PM
Unknown Object (File)
Fri, Jun 28, 4:24 PM
Unknown Object (File)
Thu, Jun 27, 7:59 AM
Unknown Object (File)
Thu, Jun 27, 7:59 AM
Unknown Object (File)
Thu, Jun 27, 7:59 AM
Unknown Object (File)
Thu, Jun 27, 7:59 AM
Unknown Object (File)
Thu, Jun 27, 7:58 AM
Unknown Object (File)
Thu, Jun 27, 7:55 AM
View All 94 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
Branch
arcpatch-D4773 (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

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 15459

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...