imay opened a new issue #3518: URL: https://github.com/apache/incubator-doris/issues/3518
After discussed in mailing list, we are going to add committed guideline for Doris Committers. Following is the draft of the guideline. **Code Review Guidelines** 1. Always maintain a high standard of review so that the quality of the entire product can be better guaranteed. 2. Changes to the architecture or the user interface need to be fully discussed by the community. This can be either in a mail group or in a GitHub issue. 3. Test coverage. The added logic needs to be covered by a corresponding test. When the old code for which there is no test is changed, this requirement can be appropriately relaxed. 4. Documentation. Newly added features must be documented, otherwise such code is not allowed. 5. Readability of the code. If reviewers are not very clear about the logic of the code, then you can ask the contributor to explain the logic. And writing sufficient comments in the code to explain the logic is encouraged. 6. Try to give a clear conclusion at the end of your comments, “approve” or “change request”. If it's a minor issue, you can just leave a comment. 7. Recommend to submit a "+1 Comment" rather than a "+1 Approve" to indicate that it looks good to me but I am not sure whether this part of the function is correct. It needs someone else's approve. 8. Respect each other and learn from each other. Maintain a polite tone when commenting, try to give reasons for the suggestions. **Pull Request Guidelines** 1. During a PR, there are three types of roles. Contributor: the PR submitter; Reviewer: the person who needs to make code-level review on the PR; and Moderator: the person responsible for coordinating the entire PR process. For example, the moderator should assign reviewers, push the author to make changes according to the comments, set different tags for the PR, and merge the PR etc. For a specific PR, a person may act in different roles, such as when a committer submitted one PR, he may himself be both the contributor and the moderator of that PR. 2. Committers can assign a PR to itself as a moderator. After assigning it to themselves, the other committer will know that this PR has been in charge. 3. The committer is encouraged to act as a moderator for its own PR. 4. The reviewer needs to perform a code-level review, refer Code Review Guidelines. 5. Once a reviewer has commented on a PR, they need to keep following up on subsequent changes to that PR. 6. A PR must get at least a +1. 7. After the first +1 to the PR, wait at least one working day before merging. The main purpose is to wait for the rest of the community to come to review. 8. For architecture or user interface changes, a PR needs to get at least 3 +1's to merge. 9. Regression cases should pass before merging. 10. Moderator needs to make sure all comments are resolved before merging. 11. Select "squash and merge" to merge. 12. When there is a disagreement about a modification, try to discuss the resolution. If the discussion doesn't work out, it can be resolved by a vote in priv...@doris.apache.org by the majority rules. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org
