Page MenuHomePhabricator
Differential D5154

[Services] Cleanup commtest/build.rs
ClosedPublic
Actions

Authored by • jon on Sep 15 2022, 3:13 PM.
Tags
None
Referenced Files
F3387777: D5154.id17047.diff
Fri, Nov 29, 11:07 AM
Unknown Object (File)
Mon, Nov 25, 9:55 PM
Unknown Object (File)
Sat, Nov 9, 6:18 PM
Unknown Object (File)
Sat, Nov 9, 7:22 AM
Unknown Object (File)
Oct 20 2024, 8:22 PM
Unknown Object (File)
Oct 10 2024, 4:16 PM
Unknown Object (File)
Oct 10 2024, 4:16 PM
Unknown Object (File)
Oct 10 2024, 4:15 PM
View All 90 Files
Subscribers
• abosh
ashoat
• karol

Details

Reviewers
varun
marcin
tomek
Commits
rCOMMa40ae16ccc03: [Services] Cleanup commtest/build.rs
Summary

Apply feedback from https://phab.comm.dev/D5102?id=16560#inline-33198.

Also:

  • Groom the file to be more idiomatic rust.
  • Ask cargo to rebuid if the protobuf files change.

D5102 was "how little do I need to do to make it build". Where as this revision
is to be a more holistic, "how would I write this file".

Test Plan
cd services/commtest
cargo build

Diff Detail

Repository
rCOMM Comm
Branch
jonringer/fix-commtest-comment
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

• jon created this revision.Sep 15 2022, 3:13 PM
Herald added subscribers: • abosh, • karol, ashoat. · View Herald TranscriptSep 15 2022, 3:13 PM
• jon mentioned this in D5102: [Services] Update commtest protobuf location.Sep 15 2022, 3:14 PM
Harbormaster completed remote builds in B12186: Diff 16716.Sep 15 2022, 3:23 PM
• jon requested review of this revision.Sep 15 2022, 3:23 PM
tomek resigned from this revision.Sep 21 2022, 4:30 AM
tomek added a reviewer: marcin.
atul removed a reviewer: atul. atul added 1 blocking reviewer(s): varun.Sep 21 2022, 5:28 AM
varun accepted this revision.Sep 21 2022, 2:24 PM

accepting with one comment

services/commtest/build.rs
12

should we warn or something here if there's a non .proto file in the protos directory?

This revision is now accepted and ready to land.Sep 21 2022, 2:24 PM
• jon marked an inline comment as done.Sep 23 2022, 11:38 AM
• jon added inline comments.
services/commtest/build.rs
12

No, we just want to pass the appropriate files to tonic. It's more of a validation pass than anything.

• jon marked an inline comment as done.Sep 23 2022, 11:40 AM
• jon added inline comments.
services/commtest/build.rs
12

Currently we have a CMakeLists.txt and a _generated directory in there. So some filtering is expected

Closed by commit rCOMMa40ae16ccc03: [Services] Cleanup commtest/build.rs (authored by • jon). · Explain WhySep 23 2022, 11:42 AM
This revision was automatically updated to reflect the committed changes.
• jon added a commit: rCOMMa40ae16ccc03: [Services] Cleanup commtest/build.rs.

Revision Contents
Changeset List

PathSize
services/
commtest/
29 lines

Diff 16716

View Options

services/commtest/build.rs

Loading...