Hello everyone, Over time, I have observed a few patterns and I feel that there are certain areas where we can collectively improve on. Following is the list of guidelines that I have put together.
- Don't commit patches without docs. - Don't commit patches without test-cases - Ensure release notes are updated in JIRA wherever applicable. *[new]* Please don't use / indulge separate JIRAs for docs / tests. All code changes should go in along with tests and docs. - Don't commit unless all review comments are addressed / answered. Please ensure all review comments, including ones from other contributors (not just committers') are addressed. As much as possible, avoid creating separate JIRAs for review comments. - Encourage reuse of pull requests Sometimes contributors unintentionally create a new pull request instead of updating the old one where the review comments were provided. Contributors should strive to reuse the old prs in order to preserve the history of reviews. Committers should try to check that prs are reused and old feedback is addressed. - Avoid putting meaningless squashed commit messages in the detailed commit message. The pr-merge tool has an option to list the squashed commit messages as part of the detailed commit message. Most pull requests have meaningless intermediate commit messages, please don't include them in the detailed commit message. However, if commit messages add valuable context, please include them. May be we should change the default option to no in the tool. - Give others a chance to review. IMHO, it is polite to wait at least 24 Hrs after a +1 before committing a change. If yours is first +1 and you intend to commit it, please consider putting a comment expressing the same. This gives others who may have some opinions on the pull request, but couldn't get to it before you, a chance to review. - Update fixVersions and update thoughtfully. Again using pr-merge tool here helps ensuring that fixVersion is updated, though figuring out correct fixVersion requires some explanation. Simple rule of thumb is - change belongs to the earliest release in which it got committed e.g. if a release branch has been cut and a commit is applied to both master and release branch, fixVersion should be release version (and not trunk). - Commit changes to all applicable branches A common mistake is to commit the patch only to the branch and not to master. Although, this has been reduced a lot by the pr tool, but it is still happening and something to keep in mind when you are not using the pr-merge tool. Just to clarify, this list is not meant to serve as list of rules but as a set of guidelines. Like all good things, these are not expected to be followed blindly. Part of being a committer is to develop the art of knowing when to make an exception. A simple comment explaining your rationale goes a long way ;) As usual, suggestions / modifications / addenda are welcome. Regards Ajay Yadava
