> On Mon, May 11, 2015 at 3:47 PM, Sean Busbey <[email protected]> wrote: >> Usually, when commits are better suited to be separate it means that there >> should be additional jiras that are sub-tasks.
+1 On Mon, May 11, 2015 at 8:51 PM, Joe Witt <[email protected]> wrote: > I was thinking the same thing Sean. > > That said I think some of these things are 'advisories to make it > easier to bring in commits'. But I'd be happy to not have them as > rules. ...conservative in what we do and liberal in what we accept... > > On Mon, May 11, 2015 at 3:47 PM, Sean Busbey <[email protected]> wrote: >> Usually, when commits are better suited to be separate it means that there >> should be additional jiras that are sub-tasks. >> >> On Mon, May 11, 2015 at 2:18 PM, Ryan Blue <[email protected]> wrote: >> >>> I don't typically like to squash a series of commits, though I do >>> typically insist that commits are sensible before merging them. >>> >>> The problem with squashing is that you end up with larger commits that are >>> more difficult to rebase or cherry-pick because they include more changes >>> than necessary. I'd much rather see a handful of focused commits that are >>> easy to reason about so when a cherry-pick conflict happens I have to worry >>> about fewer files. >>> >>> That relies on a PR being made of a few "sensible" commits as I said >>> above, which basically means no "merge master into branch X" and requires >>> the submitter to force-push to their branch. I like to keep it that way, >>> but squashing avoids this problem. >>> >>> rb >>> >>> >>> On 05/07/2015 12:45 AM, Joey Echeverria wrote: >>> >>>> I'm definitely a big fan of squashing before merging into develop. On >>>> other projects, we usually don't bother squashing before the initial >>>> PR, but I can see the advantage. After the first round of reviews has >>>> happened it is nice to see diffs between the last state and the >>>> current state so I try to avoid squashing again until the end. >>>> >>>> Generally when you squash or rebase you have to do a force commit to >>>> that branch. That shouldn't be a big problem if the branch isn't >>>> shared. If you're working on a larger feature that has multiple JIRAs >>>> all in the same branch then you have to be more careful with force >>>> pushes. >>>> >>>> -Joey >>>> >>>> On Wed, May 6, 2015 at 5:03 PM, Sean Busbey <[email protected]> wrote: >>>> >>>>> A big reason I ask for squash commits is that it makes things much easier >>>>> once we have to maintain multiple release trains. It's just easier to get >>>>> in the habit now while we're small. >>>>> >>>>> It's similar to the issue of making sure each issue has a jira reference >>>>> in >>>>> it. When I need to ask the question "is this issue fixed in release X" >>>>> it's >>>>> way way easier if I can know that when I see a commit with the jira, that >>>>> commit represents things are done rather than an incremental step. >>>>> >>>>> Generally I use the short hand "one commit, one jira" for the combination >>>>> of making sure every commit references a jira and each jira can be seen >>>>> as >>>>> fixed by a commit. Taking this to the extreme of only allowing one commit >>>>> likely isn't wise. I've been on a few projects that will go for addendums >>>>> for a short window (say 24 hours) so long as they call out e.g. "NIFI-XXX >>>>> ADDENDUM forgot to use git-add while resolving a conflict." The thinking >>>>> is >>>>> that the addendum will likely be close in the history, will be uncommon, >>>>> and if the issue is backported afterwards to some other branch it can be >>>>> squished. >>>>> >>>>> One related thing I saw recently was someone referenced the wrong jira in >>>>> their commit (or forgot the jira all together). It's unfortunate, but the >>>>> best option for this I've seen was on HBase: just revert the commit and >>>>> commit one with the correct information. Long term is eases more >>>>> headaches >>>>> then e.g. relying on a note in the jiras or something like that. >>>>> >>>>> On Wed, May 6, 2015 at 7:42 AM, Joe Witt <[email protected]> wrote: >>>>> >>>>> Dan, >>>>>> >>>>>> Yeah I was wondering about that too. I suspect it is to make the >>>>>> visual code review of the PR easier to digest. >>>>>> >>>>>> We can document it on the Github/PR language but we also need to >>>>>> produce a nice contribution guide that helps folks whether they're >>>>>> coming from Github, are in the PPMC, and so on. >>>>>> >>>>>> Thanks >>>>>> Joe >>>>>> >>>>>> On Wed, May 6, 2015 at 10:39 AM, Dan Bress <[email protected]> >>>>>> wrote: >>>>>> >>>>>>> +1 on all this stuff. >>>>>>> >>>>>>> As a side note, I noticed on a PR Busbey suggesting that people squash >>>>>>> >>>>>> their commits before submitting a PR. I know I haven't been doing this, >>>>>> and wouldn't mind hearing some pros/cons to doing this. Whatever we >>>>>> decide, can we include some language in the git hub / pull request >>>>>> section >>>>>> that describes the suggested approach. >>>>>> >>>>>>> >>>>>>> Dan Bress >>>>>>> Software Engineer >>>>>>> ONYX Consulting Services >>>>>>> >>>>>>> ________________________________________ >>>>>>> From: Mark Payne <[email protected]> >>>>>>> Sent: Wednesday, May 6, 2015 10:22 AM >>>>>>> To: [email protected] >>>>>>> Subject: Re: Consistency with Git >>>>>>> >>>>>>> I vote +1 for these suggestions. I typically name feature branches >>>>>>> after >>>>>>> >>>>>> the feature, rather than the ticket. But I can see the advantage to >>>>>> having >>>>>> the ticket name in there as well. I like the suggestion to include both. >>>>>> >>>>>>> >>>>>>> >>>>>>> For release branches, I think this will help a lot. I’ve created and >>>>>>> >>>>>> destroyed several release branches trying to get this release done. >>>>>> Having >>>>>> the common parent would ensure isolation from develop but still allow >>>>>> me to >>>>>> easily destroy the branch rather than doing a release:rollback and >>>>>> hoping >>>>>> that all works well. >>>>>> >>>>>>> >>>>>>> >>>>>>> Thanks >>>>>>> >>>>>>> -Mark >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> From: Joe Witt >>>>>>> Sent: Wednesday, May 6, 2015 9:57 AM >>>>>>> To: [email protected] >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> Team, >>>>>>> >>>>>>> It's been a few months and we started out with some ideas on how to do >>>>>>> things and each of us interpreted that slightly differently. That >>>>>>> will continue to be true but we should document these things and keep >>>>>>> getting better and more consistent. One area to consider then is the >>>>>>> usage of Git and our adherence to the Gitflow [1] workflow as we had >>>>>>> discussed. >>>>>>> >>>>>>> In my opinion there is little call for us to deal with maintenance >>>>>>> branches. So here i am just talking about 'feature branches' and >>>>>>> 'release branches'. What I'd propose is that we use what we've >>>>>>> learned and provide some better guidance on how to name the branches >>>>>>> and the life-cycle of them. >>>>>>> >>>>>>> For 'feature branches': >>>>>>> >>>>>>> - Recommend naming them 'NIFI-XYZ[-description]' The [-description] >>>>>>> would be optional. But for example this means the 'ListHDFS' branch >>>>>>> we have would have been NIFI-553-ListHDFS. >>>>>>> >>>>>>> - This naming scheme helps people to know precisely which branch that >>>>>>> is about and it also promotes cohesive feature branches (that are >>>>>>> about a particular JIRA). >>>>>>> >>>>>>> - Once the feature is complete and has been reviewed it can then be >>>>>>> merged into develop >>>>>>> >>>>>>> - It isn't clear to me when it is the right time to clean up these >>>>>>> branches. In my mind it seems like once the feature branch has been >>>>>>> merged to develop and part of a release then the feature branch can be >>>>>>> removed. It isn't necessary for Git itself to do this but seems like >>>>>>> good housekeeping. >>>>>>> >>>>>>> For 'release branches' >>>>>>> >>>>>>> - Recommend naming them 'release-nifi-X.Y.Z'. >>>>>>> >>>>>>> - This release branch would live on forever. When generating an RC it >>>>>>> should branch from that release branch. This way as RC's may come and >>>>>>> go we're not polluting the commit history. >>>>>>> >>>>>>> - Once a given RC is accepted it can be merged back to the release >>>>>>> branch, master, and develop >>>>>>> >>>>>>> - This extra sub-branch for the RC sounds a bit like overkill. But it >>>>>>> exists to ensure that we do not pollute the commit history of the >>>>>>> release and beyond but also to ensure the community can keep >>>>>>> progressing with the develop branch. >>>>>>> >>>>>>> - If for any reason we had to do an emergency type patch to a release >>>>>>> or whatnot we could do so with this branch and/or we can use the tag >>>>>>> which gets generated during the release process. >>>>>>> >>>>>>> There needs to be more discussion around the entire lifecycle of >>>>>>> contribution of code that considers all roles of the community >>>>>>> including those submitting PRs from Github. But this initial note is >>>>>>> just to get some consistency and open up for discussion. >>>>>>> >>>>>>> [1] >>>>>>> >>>>>> >>>>>> https://www.atlassian.com/git/tutorials/comparing-workflows/gitflow-workflow >>>>>> >>>>>>> >>>>>>> >>>>>>> Thanks >>>>>>> Joe >>>>>>> >>>>>> >>>>>> >>>>> >>>>> >>>>> -- >>>>> Sean >>>>> >>>> >>> >>> -- >>> Ryan Blue >>> Software Engineer >>> Cloudera, Inc. >>> >> >> >> >> -- >> Sean
