diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c7c4a61a4..0e869d353 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -43,15 +43,46 @@ Help us review your PRs more quickly by following these guidelines. - Try not to touch a large number of files in a single PR if possible. +- Provide a clear description of the motivation of your PR, this is a large + project, so context is important + - Don't change whitespace or line wrapping in parts of a file you are not editing for other reasons. Make sure your text editor is not configured to automatically reformat the whole file when saving. - Reviewers will check the staging site and contact you to fix eventual problems. +- If you agree with the suggested comment, just resolve or emoji it. No need to write a confirmation. +The code owner's mailbox will thank you later :D + If you can think of other ways we could streamline the review process, let us know. +## Pull review guidelines + +For those wanting to contribute on the quality of the incoming code, try to follow these +suggestions for a happy community =] + +- Be suggestive, never impose a correction or criticize your peer. Instead of "change this code", +go for more like "what do you think about implementing this way?" + +- Explain why you suggested such correction, a change without meaning might not be productive. +The author has all the right to counterargue a comment if she thinks it is for the best of the project. +Provide a clear technical or business justification and even links or references if possible. +Everybody loves to learn something new about coding to become a better developer. + +- Sometimes the literal answer might not be necessary. Instead of pasting the solution _in verbatim_, +provide the right direction and let the author figure out. + +- Always have a sense of community and try to help others, because they are trying to help us. + +- If a debate gets more heated in a review, try to set a call or meeting to clarify points. Letting it get out of hand +might affect the Ritchie community in general. + +- Requesting changes sometimes is perceived as a harsh action for some engineers. Try to do it with parsimony, +usually when you spot a production-breaking change or to prevent an already approved PR to be merged without +that last important touch on code. + ## Style guide Ritchie does not currently maintain a style guide. Use your best judgment, and @@ -65,3 +96,4 @@ To run: make unit-test: make functional-test: ``` +