Agree! My suggestions are not all or nothing. They are a set of ideas of which we can pick some. In the cases of multiple commits, I would suggest that we try to make it that all of them start with the same title STORM-XXX, such that it’s easily eye catching. That’s what lead to my suggestion of having a default token like STORM-MINOR when there is no JIRA associated with the patch, such that we can “group” changes and easily track them down. However, if this is too much of a requirement, I am OK with skipping it.
Hugo > On Jan 19, 2017, at 9:00 PM, Harsha Chintalapani <m...@harsha.io> wrote: > > I am ok with adopting templates but lets keep this process simpler. > We've new contributors coming in and they probably didn't have chance to go > through process. > We can guide them through the process or have strict template that everyone > needs to adopt to. > I don't think having guidelines such as having exactly 1 space after > the description is helpful to anyone. Its just a PR as long as the title > reflects the JIRA title and > JIRA is descriptive enough we all understand whats the patch intends do. > We shouldn't be going through loops for opening a PR with > restrictions such as above. > Hugo, > "STORM-XXX: Title matching exactly the JIRA Summary (title)" this > looks good to me. But having description details is hard to follow and > unnecessary. > regarding Squashing guidelines, we've gone through this discussion and I > think this should be left to author discretion as they would understand > what commits are more important. > Also as reviewers one can suggest to squash the commits if additional > commits doesn't make sense. > > Thanks, > Harsha > > On Thu, Jan 19, 2017 at 10:29 AM Hugo Da Cruz Louro <hlo...@hortonworks.com> > wrote: > >> 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 <ptgo...@gmail.com<mailto: >> ptgo...@gmail.com>> 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 <kabh...@gmail.com<mailto: >> kabh...@gmail.com>> 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) >> >> >>
