-
Notifications
You must be signed in to change notification settings - Fork 63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
style: modify parallel domain metadata with make build-proto
#102
style: modify parallel domain metadata with make build-proto
#102
Conversation
+reviewer:@tk-woven +reviewer:@kuanleetri |
c6c9aa9
to
2ae837c
Compare
I edited the commit message to follow linter expectations |
make build-proto
make build-proto
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! The generated file was built from the source tree as-is, no local changes?
We can probably extend DGP CI to build the protos and make sure everything us up to date as one of the checks.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @kuanleetri)
Yup, exactly!
That sounds cool! The CI failed. I don't believe the test is related to PD metadata though. Temporary failure (since it actually passed before; twice!)?
|
Re-running now! |
Thank you, the coverage test passed. Please re-review 🙇♀️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @kuanleetri)
I'll leave it up to you and @kuanleetri to merge this, thanks! |
Got it! I'm not very familiar with this process yet. @kuanleetri , I'd like your input if you have a moment. If some time passes and I can study this process for myself, I'll merge it on my own :) |
@tk-woven Btw could you please support here and check the merge process/reach out to @kuanleetri ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we understand the merge process. What we're asking for here is Kuan's review since I'm not familiar with this part of the development. I will need to take some time to familiarize myself, but that has to wait for some other work I'm in the middle of. If you need to reach Kuan, please kindly try Slack (I have no special powers in this regard).
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @kuanleetri)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nehal, I ran through the steps myself. Your work here LGTM! Can you please update your branch? Then when CI passes I can merge :)
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @kuanleetri)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @kuanleetri)
2ae837c
to
4cce8c4
Compare
Rebased off of master. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @kuanleetri)
I made a virtual environment following
pip install -r requirements-dev.txt
(Also tested this in the virtual environment following this.)
Although not documented in CONTRIBUTING (fixed now with #103), one of the former DGP maintainers asked to run
make build-proto
on this PR.When running make build-proto, I saw a previous commit I made be formatted differently.
This PR commits the affected file with the output of
make build-proto
so that other users don't see a file unrelated to their PR also show up in theirgit status
when contributing to this repository.This change is