falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Srikanth Sundarrajan <srik...@hotmail.com>
Subject RE: [DISCUSS] Commit guidelines
Date Sun, 04 Sep 2016 07:49:45 GMT
There are generally good set of guidelines and should make its way to https://cwiki.apache.org/confluence/display/FALCON
under the Committers guide I feel.
RegardsSrikanth Sundarrajan

> From: ajayyadava@apache.org
> Date: Sat, 3 Sep 2016 18:37:34 +0000
> Subject: [DISCUSS] Commit guidelines
> To: dev@falcon.apache.org
> 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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message