falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ajay Yadava <ajayyad...@apache.org>
Subject [DISCUSS] Commit guidelines
Date Sat, 03 Sep 2016 18:37:34 GMT
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.

Ajay Yadava

  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message