This repository was archived by the owner on Jul 11, 2023. It is now read-only.
docs(*): add pull request review guide #4111
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shashankram
reviewed
Sep 14, 2021
docs/pull_request_review_guide.md
Outdated
| - Leave comments if the PR author can make adjustments to write better Go according to the [Go style guide](https://golang.org/doc/effective_go). | ||
| - Ask questions if the code does not make sense to you. | ||
| - Make sure each PR includes the appropriate unit tests and e2e tests and read the tests to make sure you understand what the author is changing and what is the expected behavior of the code. | ||
| - Remind the author to squash commits into a single commit where it makes sense. Each commit merged into the main branch should be able to pass all CI tests. |
There was a problem hiding this comment.
Squashing is not required anymore. PR authors can request maintainers to squash their commits before merging. We established that this is standard practice across many public projects, including Envoy, Contour etc.
https://github.com/openservicemesh/osm/blob/main/CONTRIBUTING.md#merging-pull-requests
There was a problem hiding this comment.
thanks. I updated the text to reflect this. Please let me know if you have a different suggestion.
+ add README in docs dir + remove redundant demo + rename dev_guide dir to development_guide resolves openservicemesh#1434 Signed-off-by: Michelle Noorali <[email protected]>
shashankram
approved these changes
Sep 14, 2021
shalier
approved these changes
Sep 15, 2021
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
resolves #1434
Signed-off-by: Michelle Noorali [email protected]
Please mark with X for applicable areas.
-->
Affected area:
Please answer the following questions with yes/no.
Does this change contain code from or inspired by another project? No
Is this a breaking change? No