Skip to content
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

docs: add instructions for protos contribution #103

Merged

Conversation

nehalmamgain
Copy link
Contributor

@nehalmamgain nehalmamgain commented Aug 1, 2022

Clarify an important workflow in CONTRIBUTING.md to avoid users adding compiled protos through their own workflows.
Example: related PR with above issue.


This change is Reviewable

@nehalmamgain
Copy link
Contributor Author

How do I run the linter locally before committing this doc change?
Is there a doc on the repo I can refer for that?

Copy link
Collaborator

@tk-woven tk-woven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this! I poked around but did not find docs for any single linting entrypoint. Probably a feature we could add for developer convenience.

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @nehalmamgain)


docs/CONTRIBUTING.md line 81 at r1 (raw file):

**NOTE:** Any [proto schema](../dgp/proto) changes must be seperated from code changes into an independent commit and PR.

Also, please ensure to commit the compiled protos to the aformentioned PR by first running `make build-proto`. 

Thanks! Can you please make this part of the **NOTE:** paragraph? Also, let's reword a little to suggest that we need to run make build-proto and then include the artifacts it produces in the commit and PR.

@nehalmamgain
Copy link
Contributor Author

Thanks Tyler, I will push your requested changes later today.

@nehalmamgain nehalmamgain force-pushed the docs/nehal/add-instruction-for-protos branch from 92e22a8 to bd08592 Compare August 3, 2022 03:48
Copy link
Collaborator

@tk-woven tk-woven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @nehalmamgain)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants