Paths

Table of Contentst

[Services] Cleanup commtest/build.rs
ClosedPublic
Actions

Authored by • jon on Sep 15 2022, 3:13 PM.

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

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

Path

Size

services/

commtest/

build.rs

29 lines

Diff 17047

View Options

services/commtest/build.rs

[Services] Cleanup commtest/build.rsClosedPublicActions