I am strongly +1. In addition to the templates suggested by Jungtaek, in the 
past I suggested the template bellow to a different project. It is a bit 
simpler than the ones listed, as I think it may be too demanding to ask people 
to categorize in the git message the type of contribution the patch is 
addressing (BUG, Feature, …), if it has docs, if it has tests, etc. I also 
think that it may make the git message too verbose. That info is contained in 
the JIRA, and if there is a 1 to 1 mapping between Git commit and JIRA, it’s 
fairly easy to track down.

The PR template is important, but more important, in my opinion, is to have 
strong guidelines requiring that people squash commits within functional 
units/features the code is addressing. Furthermore, if the commits are for one 
JIRA/change (when no JIRA), and there are more than one commit, all should have 
the same title (STORM-XXX) in it. We should at all costs avoid merging into 
master extreme cases like 10 commits related to one patch, where 5 or 6 of them 
are commits along the lines, addressed code review, added tests, cleaned up 
tests, addressed more code reviews, etc... without even having the STORM-XXX 
prefix in them.

Squashing commits not only will make cherry-pick much easier, but more 
importantly, if there is a regression, it is very easy to track down which 
change is causing the regression. Not doing this makes it very hard to track 
down which change is causing problems.

If we agree on some guidelines, I volunteer to update the documentation.

=== GIT TEMPLATE GUIDELINES ===

STORM-XXX: Title matching exactly the JIRA Summary (title)

- An optional, bulleted (+, -, ., *), description of the contents of
- the patch. The goal is not to describe the contents of every file,
- but rather give a quick overview of the main functional areas
- addressed by the patch.

The text immediately following the JIRA number (STORM-XXX: ) must be an exact 
transcription of the JIRA summary (title), not the a summary of the contents of 
the patch. If the JIRA summary does not accurately describe what the patch is 
addressing, the JIRA summary must be fixed, and then copied to the Git commit 
message.

A description with the contents of the patch is optional but strongly 
encouraged if the patch is large and/or the JIRA title is not expressive enough 
to highlight what the patch is doing. This text must be added bellow, with one 
empty line of space in between, and be bulleted using one of the following 
bullet points (+, -, ., *). There must be at last a 1 space indent before the 
bullet char, and exactly one space between bullet char and the first letter of 
the text. Bullets are not optional but required.

=== GIT TEMPLATE GUIDELINES FOR COMMITS WITHOUT JIRA ===

For the sake of having a pattern one could grep, or exclude from grep, I 
suggest that we prefix every commit that does not have a JIRA associated with 
it with something along the lines:

STORM-MINOR: Title summarizing what the patch is addressing.

If a description of the change is necessary, it should follow the guidelines 
above. The squashing guidelines also apply.

=== SQUASHING GUIDELINES ===

- Each commit message should have the first line matching the associated JIRA 
(STORM-XXX, or STORM-MINOR) as described
- If possible, one JIRA/change should be one commit.
- If the patch is large, and it makes sense to break it into multiple commits 
to make it easier to understand and cherry-pick (not easy to merge), each 
commit should represent the functional units/features the code is addressing
- During code review it is OK to have multiple commits addressing the changes 
proposed by the reviewers (also because it makes it easy to track those 
changes), but once the patch is ready to merge (has enough +1’s), the commits 
should be squashed according to the guidelines.

Thanks,
Hugo

On Jan 18, 2017, at 6:15 PM, P. Taylor Goetz 
<[email protected]<mailto:[email protected]>> wrote:

+1 for adopting a template. Not sure if it is possible, but I wouldn't mind 
eyeing the same for JIRA.

-Taylor

On Jan 18, 2017, at 9:10 PM, Jungtaek Lim 
<[email protected]<mailto:[email protected]>> wrote:

Hi devs,

I have seen some pull requests which are not having JIRA issue, or bad
title, or no description for rationale on pull request and issue.

We already have DEVELOPER.md but this document covers more than
contribution and also not shown while opening pull request.

I think we should add pull request template guiding how to create pull
request properly. Ideally it can be detailed form but at least it should
guide about title, and target branch.

Some examples are here:

https://github.com/apache/spark/blob/master/.github/PULL_REQUEST_TEMPLATE
https://github.com/apache/flink/blob/master/.github/PULL_REQUEST_TEMPLATE.md
https://github.com/apache/zeppelin/blob/master/.github/PULL_REQUEST_TEMPLATE
https://github.com/apache/incubator-gearpump/blob/master/.github/PULL_REQUEST_TEMPLATE.md

What's your opinion?

Thanks,
Jungtaek Lim (HeartSaVioR)


Reply via email to