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
F3561180: D5154.id17047.diff
Fri, Dec 27, 8:38 AM
F3561178: D5154.id16716.diff
Fri, Dec 27, 8:38 AM
F3561176: D5154.diff
Fri, Dec 27, 8:38 AM
F3558722: D5154.id17047.diff
Fri, Dec 27, 5:05 AM
F3558721: D5154.id16716.diff
Fri, Dec 27, 5:05 AM
F3558695: D5154.id.diff
Fri, Dec 27, 5:04 AM
F3558681: D5154.diff
Fri, Dec 27, 4:56 AM
Unknown Object (File)
Sun, Dec 1, 3:19 PM
View All 98 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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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 ↗(On Diff #16716)

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 ↗(On Diff #16716)

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 ↗(On Diff #16716)

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 17047

View Options

services/commtest/build.rs

Loading...