This is the approach I am using as well Blake. Thanks, Mark
> On Jan 2, 2020, at 7:16 AM, Blake Bender <bben...@pivotal.io> wrote: > > That's not how I approach this sort of a change, and to my mind it feels > sort of like that approach is asking for trouble. When I have a refactor > plus a code change, I do the refactor on a branch, submit the PR, then > branch off of *that* branch to do the feature work. This forces an > ordering of the changes, but alleviates most/all of the conflicts. When > the refactor PR is accepted, I rebase the feature work off of the new > develop branch, and submit the second PR, usually without encountering any > issues. > > > > On Wed, Jan 1, 2020 at 9:57 AM Aaron Lindsey <aaronlind...@apache.org> > wrote: > >>> >>> Is it not the case currently? If I am working on a feature modifying >> class >>> X and another developer makes some refactoring changes on class X and >>> pushes it to develop, won't I have to resolve the merge commits anyway. >> >> >> Yes, you will always have to deal with resolving conflicts with other >> people's changes. The scenario I was describing was me having to resolve >> conflicts between my own 2 changes that modify the same files. If I make a >> refactor commit and a fix commit as two separate PRs that are each based on >> develop (i.e. they are independent PRs), after I merge the first one to >> develop the second one will have merge conflicts. The simplest way to avoid >> this is to put the commits in the same PR and use rebase-and-merge. >> >> >> >> On Tue, Dec 31, 2019 at 10:46 PM Owen Nichols <onich...@pivotal.io> wrote: >> >>> It sounds like there is value in being able to deliver un-squashed PRs, >> and >>> we believe the richer commit message history outweighs any potential >>> inconvenience to bisect operations (as Aaron pointed out, finer-grained >>> commits should generally be to the benefit of bisect operations). >>> >>> We will leave all 3 merge options enabled in GitHub. >>> >>> On Tue, Dec 31, 2019 at 7:27 PM Dan Smith <dsm...@pivotal.io> wrote: >>> >>>> I also tend to do the same workflow Aaron described - make a >> refactoring >>>> change to support a feature followed by the actual change I want to >> make. >>>> It makes the history a lot clearer because refactoring tends to touch a >>> lot >>>> of files, so it's nice to have those changes clearly marked as a >>>> refactoring that should not change behavior. >>>> >>>> It's possible to do this as to separate PRs, but that drags out the >>> process >>>> because you have to get the first in merged before you create the >> second. >>>> That encourages bunching changes in a single squashed commit, which >> makes >>>> git blame less useful. >>>> >>>> >>>> -Dan >>>> >>>> On Tue, Dec 31, 2019, 6:36 PM Nabarun Nag <n...@apache.org> wrote: >>>> >>>>> Aaron , >>>>> >>>>> Is it not the case currently? If I am working on a feature modifying >>>> class >>>>> X and another developer makes some refactoring changes on class X and >>>>> pushes it to develop, won't I have to resolve the merge commits >> anyway. >>>>> >>>>> >>>>> Regards >>>>> Naba >>>>> >>>>> >>>>> On Tue, Dec 31, 2019 at 6:01 PM Aaron Lindsey < >> aaronlind...@apache.org >>>> >>>>> wrote: >>>>> >>>>>> Suppose I’m refactoring the same classes I’m touching for the >>> feature. >>>> I >>>>>> do the refactoring on one branch and submit a PR for that. Then I >>>>> implement >>>>>> the feature on another branch which is based on develop and does >> not >>>> have >>>>>> the refactoring changes since those are not merged yet. I’ll have >> to >>>>>> resolve conflicts on the second PR that I merge. >>>>>> >>>>>> Having interdependent PRs where one PR branch is based on another >> PR >>>>>> branch is not something I’ve tried. That requires more overhead in >>>> having >>>>>> to create more PRs and branches. And if you have N interdependent >>> PRs, >>>>> you >>>>>> have to do N-1 merges each time a PR is merged to develop (to >> update >>>> the >>>>>> other branches). It could be done, but is it worth the hassle? >>>>>> >>>>>> One more point about rebase-and-merge is that it seems like this >>> would >>>>>> make bisecting a failure easier since there would be smaller >> commits. >>>>>> >>>>>> Aaron >>>>>> >>>>>>> On Dec 31, 2019, at 5:41 PM, Owen Nichols <onich...@pivotal.io> >>>> wrote: >>>>>>> >>>>>>> Can you elaborate on why you would have to deal with conflicts if >>> the >>>>>>> refactoring work was kept as a separate PR from the fix? >>>>>>> >>>>>>> As with many git workflows, there’s an easy way and a hard way to >>>>> manage >>>>>>> interdependent PRs (tl;dr only merge, never rebase!). I wonder if >>>> this >>>>>>> points out an opportunity for a “tips and tricks” page on the >> Geode >>>>> wiki. >>>>>>> >>>>>>> On Tue, Dec 31, 2019 at 5:22 PM Aaron Lindsey < >>>> aaronlind...@apache.org >>>>>> >>>>>>> wrote: >>>>>>> >>>>>>>> -0.9 >>>>>>>> >>>>>>>> I’m not in favor of the revised proposal that disallows >>>>>> rebase-and-merge. >>>>>>>> Say I am working on a PR and I have a refactor commit and >> another >>>>> commit >>>>>>>> which implements a new feature. I don’t want those commits to >> get >>>>>> squashed >>>>>>>> because that makes it hard to understand the diff. However, if I >>>> make >>>>>> those >>>>>>>> commits as two separate PRs then I am going to have to deal with >>>>>> conflicts. >>>>>>>> >>>>>>>> I’m not sure when we made it a rule that every commit in develop >>> had >>>>> to >>>>>>>> compile and pass tests. I know we implemented a rule that all >> PRs >>>> had >>>>> to >>>>>>>> pass certain checks, but I never thought that rule implied all >>>> commits >>>>>> had >>>>>>>> to pass those checks. >>>>>>>> >>>>>>>> In general I just don’t see the problem with rebase-and-merge >> and >>>> this >>>>>>>> feels like an unnecessary restriction, but I will go with it if >>>> that’s >>>>>> what >>>>>>>> everyone wants to do. >>>>>>>> >>>>>>>> Aaron >>>>>>>> >>>>>>>>> On Dec 31, 2019, at 3:09 PM, Owen Nichols <onich...@pivotal.io >>> >>>>> wrote: >>>>>>>>> >>>>>>>>> To recap, this proposal is now revised to remove 2 of the 3 >> merge >>>>>> options >>>>>>>>> from GitHub, leaving only Squash and Merge. PR #4513 >>>>>>>>> <https://github.com/apache/geode/pull/4513> serves as an >> exhibit >>>> of >>>>>>>> what is >>>>>>>>> proposed (it is not to be merged unless discussion leads to >>>> consensus >>>>>>>> and a >>>>>>>>> successful vote). Please use the dev list (not the PR) for all >>>>>>>> discussion >>>>>>>>> (and voting, if we get that far). >>>>>>>>> >>>>>>>>> Squash and merge is already used almost exclusively by the >> Geode >>>>>>>> community, >>>>>>>>> with any exceptions tending to be accidental more often than >>>>>> intentional. >>>>>>>>> However, some have raised the concern that implementing this >>>>>> restriction >>>>>>>>> could result in harm or wasted time. Can someone give an >> example >>>> of >>>>> a >>>>>>>> such >>>>>>>>> a scenario? >>>>>>>>> >>>>>>>>> It seems there is a divide here between junior and senior >>> community >>>>>>>>> members. Newer committers appreciate additional guardrails to >>>>> protect >>>>>>>> them >>>>>>>>> from accidentally doing the wrong thing, while those with more >>>>>> experience >>>>>>>>> want to be able to work unencumbered by restrictions of any >> kind. >>>>>>>>> >>>>>>>>> Our welcome email to new committers states “We like to work on >>>> trust >>>>>>>> rather >>>>>>>>> than unnecessary constraints...Being a committer enables you to >>>> more >>>>>>>> easily >>>>>>>>> make changes without needing to go through the patch submission >>>>>> process”. >>>>>>>>> I can see this as an argument against this proposal (perhaps >> even >>>> an >>>>>>>>> argument against any form of branch protection). >>>>>>>>> >>>>>>>>> In the scheme of things, this proposal makes very little >>>> difference. >>>>>>>> There >>>>>>>>> are still other ways to get non-compiling commits onto develop >>>> (e.g. >>>>>>>>> waiting a long time between running PR checks and merging to >>>>> develop). >>>>>>>>> What’s more important is working well together as a community. >>> So, >>>>>>>> perhaps >>>>>>>>> what’s best for the community is to encourage working on trust >>>> rather >>>>>>>> than >>>>>>>>> unnecessary constraints. >>>>>>>>> >>>>>>>>> -Owen >>>>>>>>> >>>>>>>>> On Dec 31, 2019, at 10:56 AM, Kirk Lund <kl...@apache.org> >>> wrote: >>>>>>>>> >>>>>>>>> I'm happy to file multiple PRs when I need to merge multiple >>>> commits >>>>> to >>>>>>>>> develop. >>>>>>>>> >>>>>>>>> On Mon, Dec 30, 2019 at 5:45 PM Mark Hanson < >> mhan...@pivotal.io> >>>>>> wrote: >>>>>>>>> >>>>>>>>> This change to disable all but squash-merge would be really >> easy >>> to >>>>>>>>> revert. How about we try it for a while and see? If people >> decide >>>> it >>>>> is >>>>>>>>> really limiting them, we can change it back. Let’s do it for 1 >>>> month >>>>>> and >>>>>>>>> see how it goes. Does that sound reasonable? >>>>>>>>> >>>>>>>>> Thanks, >>>>>>>>> Mark >>>>>>>>> >>>>>>>>> On Dec 30, 2019, at 5:25 PM, Owen Nichols <onich...@pivotal.io >>> >>>>> wrote: >>>>>>>>> >>>>>>>>> Given that we adopted < >>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>> >>>>> >>>> >>> >> https://lists.apache.org/thread.html/c3eb5c028cb3a4d76024f928a7a33c0311228f5dbbcaa86287bf5826@%3Cdev.geode.apache.org%3E >>>>>>>>>> >>>>>>>>> and still wish to continue < >>>>>>>>> >>>>>>>> >>>>>> >>>>> >>>> >>> >> https://lists.apache.org/thread.html/8795697c1ad57068c053b48b4b1845005f3ade0be777e679eafe95db@%3Cdev.geode.apache.org%3E >>>>>>>>>> >>>>>>>>> having branch protection rules to ensure every commit landing >> in >>>>>> develop >>>>>>>>> has passed the required PR checks, it seems like that decision >>>> alone >>>>>>>>> mandates that we disable all merge mechanisms other than >>>>>>>> squash-and-merge. >>>>>>>>> >>>>>>>>> >>>>>>>>> Allowing merge commits or rebase merges bypasses branch >>> protection >>>>> for >>>>>>>>> >>>>>>>>> all commits other than the final one in the merge or rebase >> set. >>>>> Given >>>>>>>>> that we decided < >>>>>>>>> >>>>>>>> >>>>>> >>>>> >>>> >>> >> https://lists.apache.org/thread.html/1ba19d9aeb206148c922afdd182ba322d6f128bbb83983f2f72a108e@%3Cdev.geode.apache.org%3E >>>>>>>>>> >>>>>>>>> that bypassing PR checks should never be allowed, keeping this >>>>> loophole >>>>>>>>> open seems untenable. >>>>>>>>> >>>>>>>>> >>>>>>>>> This is not just hypothetical — this loophole is causing real >>>>> problems. >>>>>>>>> >>>>>>>>> We now have commits on develop that don’t compile. For >> example: >>>>>>>>> >>>>>>>>> git checkout 19eee7821322a1301f16bdcd31fd3b8c872a41b6 >>>>>>>>> ./gradlew devBuild >>>>>>>>> ...spotlessJava FAILED >>>>>>>>> We implemented branch protections to make this impossible, >> right? >>>>>>>>> >>>>>>>>> We can very easily close this loophole by disabling all but the >>>>>>>>> >>>>>>>>> Squash&Merge button for PRs. This will not make more work for >>> any >>>>>>>>> developer. If you want to get multiple commits onto develop, >>>> simply >>>>>>>> submit >>>>>>>>> each as a separate PR — that is the only way to assure that >>>> required >>>>> PR >>>>>>>>> checks have passed for each. >>>>>>>>> >>>>>>>>> >>>>>>>>> On the other hand, if we as a Geode community feel strongly >> that >>>>>>>>> >>>>>>>>> bypassing branch protection via merge commits and rebase >> commits >>> is >>>>>>>>> important to allow, why not also allow arbitrary overrides (or >>> get >>>>> rid >>>>>> of >>>>>>>>> branch protection entirely)? >>>>>>>>> >>>>>>>>> >>>>>>>>> -Owen >>>>>>>>> >>>>>>>>> On Dec 20, 2019, at 12:31 PM, Blake Bender <bben...@pivotal.io >>> >>>>> wrote: >>>>>>>>> >>>>>>>>> Just FWIW, the situation described of having multiple commits >> in >>> a >>>>>>>>> >>>>>>>>> single >>>>>>>>> >>>>>>>>> PR with separate associated JIRA tickets is still kind of >>>>> problematic. >>>>>>>>> >>>>>>>>> It >>>>>>>>> >>>>>>>>> could well be the case that the commits are interdependent, >> thus >>>> when >>>>>>>>> bisecting etc it's still not possible to revert the fix for a >>>> single >>>>>>>>> bug/feature/whatever atomically. It's all good, though, I'm >>>>> satisfied >>>>>>>>> >>>>>>>>> no >>>>>>>>> >>>>>>>>> one's forcing me to adopt practice I'm opposed to. Apologies >> for >>>>>>>>> >>>>>>>>> getting >>>>>>>>> >>>>>>>>> my feathers a little ruffled, or if I ruffled anyone else's in >>>>> return. >>>>>>>>> >>>>>>>>> Thanks, >>>>>>>>> >>>>>>>>> Blake >>>>>>>>> >>>>>>>>> >>>>>>>>> On Fri, Dec 20, 2019 at 12:18 PM Nabarun Nag <n...@pivotal.io> >>>>> wrote: >>>>>>>>> >>>>>>>>> Hi Dan, >>>>>>>>> >>>>>>>>> When we do squash merge all the commit messages are preserved >> and >>>>> also >>>>>>>>> >>>>>>>>> the >>>>>>>>> >>>>>>>>> co-authored tag works when we do squash merge. >>>>>>>>> So the authorship and history are preserved. >>>>>>>>> >>>>>>>>> In my own personal experience and reverts and pinpointing >>>> regression >>>>>>>>> failures are tough when the commits are spread out. Also, >> reverts >>>> are >>>>>>>>> easier when it is just one commit while we are bisecting >>> failures. >>>>>>>>> >>>>>>>>> >>>>>>>>> Regards >>>>>>>>> Naba >>>>>>>>> >>>>>>>>> On Fri, Dec 20, 2019 at 12:07 PM Dan Smith <dsm...@pivotal.io> >>>>> wrote: >>>>>>>>> >>>>>>>>> I'll change to -0. >>>>>>>>> >>>>>>>>> I think merge commits are a nice way to record history in some >>>> cases, >>>>>>>>> >>>>>>>>> and >>>>>>>>> >>>>>>>>> can also be a way to avoid messy conflicts that come with >> rebase. >>>>>>>>> >>>>>>>>> Merge >>>>>>>>> >>>>>>>>> commits also preserve authorship of commits (compared to >>>>>>>>> >>>>>>>>> squash-merge), >>>>>>>>> >>>>>>>>> which I think is valuable as an open source community that is >>>> trying >>>>>>>>> >>>>>>>>> to >>>>>>>>> >>>>>>>>> keep track of outside contributions. >>>>>>>>> >>>>>>>>> That said, if the rest of y'all feel it will help to disable >> the >>>>>>>>> >>>>>>>>> button, >>>>>>>>> >>>>>>>>> I >>>>>>>>> >>>>>>>>> won't stand in the way. >>>>>>>>> >>>>>>>>> -Dan >>>>>>>>> >>>>>>>>> On Fri, Dec 20, 2019 at 11:50 AM Anthony Baker < >>> aba...@pivotal.io> >>>>>>>>> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> Whether we are talking about the geode/ repository or the >>>>>>>>> >>>>>>>>> geode-native/ >>>>>>>>> >>>>>>>>> repository we are all one GEODE community. >>>>>>>>> >>>>>>>>> The idea of a native client team may matter in some contexts >> but >>> in >>>>>>>>> >>>>>>>>> this >>>>>>>>> >>>>>>>>> space we are all GEODE contributors. >>>>>>>>> >>>>>>>>> Adopting a common approach across our repos will make it easier >>> for >>>>>>>>> >>>>>>>>> new >>>>>>>>> >>>>>>>>> contributors to engage, learn our norms, and join our efforts. >>>>>>>>> >>>>>>>>> $0.02, >>>>>>>>> Anthony >>>>>>>>> >>>>>>>>> >>>>>>>>> On Dec 20, 2019, at 11:32 AM, Blake Bender <bben...@pivotal.io >>> >>>>>>>>> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> Is this a policy the native client team must abide by, as well? >>> It >>>>>>>>> >>>>>>>>> varies >>>>>>>>> >>>>>>>>> slightly with what we've been doing, and all other things being >>>>>>>>> >>>>>>>>> equal I >>>>>>>>> >>>>>>>>> see >>>>>>>>> >>>>>>>>> no reason for us to change that. If I am to have any measure >> of >>>>>>>>> >>>>>>>>> control >>>>>>>>> >>>>>>>>> over the nc repository, I will definitely enforce a 1:1 >>>>>>>>> >>>>>>>>> correspondence >>>>>>>>> >>>>>>>>> between commits to develop and JIRA tickets. IMO, if your >>>>>>>>> >>>>>>>>> refactoring >>>>>>>>> >>>>>>>>> in a >>>>>>>>> >>>>>>>>> PR is sufficiently large or complex that it's difficult to >> tease >>> it >>>>>>>>> >>>>>>>>> out >>>>>>>>> >>>>>>>>> from the bug you're fixing or feature you're implementing, it >>>> merits >>>>>>>>> >>>>>>>>> its >>>>>>>>> >>>>>>>>> own JIRA ticket and a separate PR. If your "actual" fix then >>>>>>>>> >>>>>>>>> becomes >>>>>>>>> >>>>>>>>> dependent on the refactored code, that's a price I'm willing to >>> pay >>>>>>>>> >>>>>>>>> to >>>>>>>>> >>>>>>>>> keep >>>>>>>>> >>>>>>>>> history clean. >>>>>>>>> >>>>>>>>> On the other hand, I see no real value in squashing to a single >>>>>>>>> >>>>>>>>> commit >>>>>>>>> >>>>>>>>> prior to submitting a PR, since your view of the changes on >>> GitHub >>>>>>>>> >>>>>>>>> is >>>>>>>>> >>>>>>>>> essentially the same either way. We haven't enforced this on >> the >>>> nc >>>>>>>>> >>>>>>>>> repo, >>>>>>>>> >>>>>>>>> and I'd prefer to keep it that way. >>>>>>>>> >>>>>>>>> Thanks, >>>>>>>>> >>>>>>>>> Blake >>>>>>>>> >>>>>>>>> >>>>>>>>> On Fri, Dec 20, 2019 at 10:29 AM Jinmei Liao < >> jil...@pivotal.io> >>>>>>>>> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> Merge commit is the new contributor's default setting. When we >>> are >>>>>>>>> >>>>>>>>> merging >>>>>>>>> >>>>>>>>> new contributor's PR, since we are so used to THINKING >>>>>>>>> >>>>>>>>> "squash-and-merge" >>>>>>>>> >>>>>>>>> is the default, we forgot to check what the button really says >>> when >>>>>>>>> >>>>>>>>> we >>>>>>>>> >>>>>>>>> are >>>>>>>>> >>>>>>>>> merging other people's PR. >>>>>>>>> >>>>>>>>> On Fri, Dec 20, 2019 at 10:18 AM Ernest Burghardt < >>>>>>>>> >>>>>>>>> eburgha...@pivotal.io> >>>>>>>>> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> I'm a proponent of using squash-and-merge, and once a person >> has >>>>>>>>> >>>>>>>>> chosen >>>>>>>>> >>>>>>>>> this option once it comes up by default afterwards... >>>>>>>>> >>>>>>>>> Owen, I don't think you have consensus to put forth this PR, >>>>>>>>> >>>>>>>>> there >>>>>>>>> >>>>>>>>> are >>>>>>>>> >>>>>>>>> -1s >>>>>>>>> >>>>>>>>> above... (early voting) >>>>>>>>> >>>>>>>>> maybe we'll be better off socializing the norm of >>> squash-and-merge >>>>>>>>> >>>>>>>>> and >>>>>>>>> >>>>>>>>> gaining a natural consensus that way... >>>>>>>>> >>>>>>>>> On Fri, Dec 20, 2019 at 10:07 AM Owen Nichols < >>>>>>>>> >>>>>>>>> onich...@pivotal.io >>>>>>>>> >>>>>>>>> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> The proposed action manifests as a commit to the Geode git >>>>>>>>> >>>>>>>>> repository, >>>>>>>>> >>>>>>>>> so >>>>>>>>> >>>>>>>>> a PR is an acceptable vehicle for voting in this case. >>>>>>>>> >>>>>>>>> On Dec 20, 2019, at 9:38 AM, Bruce Schuchardt < >>>>>>>>> >>>>>>>>> bschucha...@pivotal.io> >>>>>>>>> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> I see a lot of plus-ones and a "voting deadline" on this >> DISCUSS >>>>>>>>> >>>>>>>>> thread >>>>>>>>> >>>>>>>>> and a request to "vote" using a PR. This all seems out of >> order >>>>>>>>> >>>>>>>>> to >>>>>>>>> >>>>>>>>> me. >>>>>>>>> >>>>>>>>> Our votes are supposed to be on the email list, aren't they? >> and >>>>>>>>> >>>>>>>>> I >>>>>>>>> >>>>>>>>> haven't >>>>>>>>> >>>>>>>>> seen a VOTE request. >>>>>>>>> >>>>>>>>> >>>>>>>>> On 12/20/19 9:33 AM, Nabarun Nag wrote: >>>>>>>>> >>>>>>>>> +1 >>>>>>>>> >>>>>>>>> On Fri, Dec 20, 2019 at 9:25 AM Owen Nichols < >>>>>>>>> >>>>>>>>> onich...@pivotal.io >>>>>>>>> >>>>>>>>> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> Based on the feedback so far, I will amend the proposal to >>>>>>>>> >>>>>>>>> drop >>>>>>>>> >>>>>>>>> item >>>>>>>>> >>>>>>>>> 2). >>>>>>>>> >>>>>>>>> Therefore, the current ability to create merge commits using >>>>>>>>> >>>>>>>>> command-line >>>>>>>>> >>>>>>>>> git will remain available. >>>>>>>>> >>>>>>>>> The proposal as amended is now: >>>>>>>>> >>>>>>>>> Change GitHub settings to make "Squash and merge" the default >>>>>>>>> (by removing “Create a merge commit” option). >>>>>>>>> >>>>>>>>> Update the PR template to change the text "Is your initial >>>>>>>>> >>>>>>>>> contribution >>>>>>>>> >>>>>>>>> a single, squashed commit” to “Is your initial contribution >>>>>>>>> >>>>>>>>> squashed >>>>>>>>> >>>>>>>>> for >>>>>>>>> >>>>>>>>> ease of review (e.g. a single commit, or a ‘refactoring’ >>>>>>>>> >>>>>>>>> commit >>>>>>>>> >>>>>>>>> plus a >>>>>>>>> >>>>>>>>> ‘fix’ commit)" >>>>>>>>> >>>>>>>>> >>>>>>>>> As Naba suggested, we can try it, and if we don’t like it, >>>>>>>>> >>>>>>>>> it’s >>>>>>>>> >>>>>>>>> simple >>>>>>>>> >>>>>>>>> to >>>>>>>>> >>>>>>>>> revert. >>>>>>>>> >>>>>>>>> I’ve create a PR for the proposed change here: >>>>>>>>> https://github.com/apache/geode/pull/4513 >>>>>>>>> >>>>>>>>> Please use the PR to vote for against this proposal. It will >>>>>>>>> >>>>>>>>> not >>>>>>>>> >>>>>>>>> be >>>>>>>>> >>>>>>>>> merged before the VOTING DEADLINE of DEC 31 (if no -1’s at >>>>>>>>> >>>>>>>>> that >>>>>>>>> >>>>>>>>> time) >>>>>>>>> >>>>>>>>> >>>>>>>>> On Dec 20, 2019, at 8:31 AM, Ju@N <jujora...@gmail.com> >>>>>>>>> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> +1 >>>>>>>>> >>>>>>>>> On Fri 20 Dec 2019 at 16:18, Owen Nichols < >>>>>>>>> >>>>>>>>> onich...@pivotal.io> >>>>>>>>> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> Hi Bruce, this proposal will not waste a single second of >>>>>>>>> >>>>>>>>> your >>>>>>>>> >>>>>>>>> time. It >>>>>>>>> >>>>>>>>> just prevents people from accidentally pressing a button >>>>>>>>> >>>>>>>>> that >>>>>>>>> >>>>>>>>> we >>>>>>>>> >>>>>>>>> have >>>>>>>>> >>>>>>>>> already agreed should never be pressed, but because we never >>>>>>>>> >>>>>>>>> configured >>>>>>>>> >>>>>>>>> our >>>>>>>>> >>>>>>>>> GitHub to match our stated policy, is currently the default. >>>>>>>>> >>>>>>>>> However, it will save a lot of time and frustration for >>>>>>>>> >>>>>>>>> anyone >>>>>>>>> >>>>>>>>> needing >>>>>>>>> >>>>>>>>> to >>>>>>>>> >>>>>>>>> bisect failures, revert, or cherry-pick changes, which has >>>>>>>>> >>>>>>>>> merit >>>>>>>>> >>>>>>>>> even if >>>>>>>>> >>>>>>>>> you personally never do any of those three things. >>>>>>>>> >>>>>>>>> Please start a separate thread if you would like to revisit >>>>>>>>> >>>>>>>>> the >>>>>>>>> >>>>>>>>> community >>>>>>>>> >>>>>>>>> decision to require passing PR checks. >>>>>>>>> >>>>>>>>> On Dec 20, 2019, at 7:49 AM, Bruce Schuchardt < >>>>>>>>> >>>>>>>>> bschucha...@pivotal.io> >>>>>>>>> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> I agree with Jake. I would go further by saying that I see >>>>>>>>> >>>>>>>>> very >>>>>>>>> >>>>>>>>> little >>>>>>>>> >>>>>>>>> merit in this proposal. I think we're getting more and more >>>>>>>>> >>>>>>>>> bureaucratic >>>>>>>>> >>>>>>>>> in our process and that it stifles productivity. I was >>>>>>>>> >>>>>>>>> recently >>>>>>>>> >>>>>>>>> forced >>>>>>>>> >>>>>>>>> to >>>>>>>>> >>>>>>>>> spend three days fixing tests in which I had changed an >>>>>>>>> >>>>>>>>> import >>>>>>>>> >>>>>>>>> statement >>>>>>>>> >>>>>>>>> before they would pass stress testing. I'm glad the tests >>>>>>>>> >>>>>>>>> now >>>>>>>>> >>>>>>>>> pass >>>>>>>>> >>>>>>>>> reliably but I was very frustrated by the process. >>>>>>>>> >>>>>>>>> On 12/19/19 4:49 PM, Jacob Barrett wrote: >>>>>>>>> >>>>>>>>> I’m in agreement with Dan. Changes to the infrastructure >>>>>>>>> >>>>>>>>> to >>>>>>>>> >>>>>>>>> flat >>>>>>>>> >>>>>>>>> out >>>>>>>>> >>>>>>>>> prevent things that should be self policing is annoying. >>>>>>>>> >>>>>>>>> This >>>>>>>>> >>>>>>>>> PR >>>>>>>>> >>>>>>>>> review >>>>>>>>> >>>>>>>>> lock we have had already cost us valuable time waiting for >>>>>>>>> >>>>>>>>> PR >>>>>>>>> >>>>>>>>> pipelines >>>>>>>>> >>>>>>>>> to >>>>>>>>> >>>>>>>>> pass that have no relevance to the commit, like CI work: I’d >>>>>>>>> >>>>>>>>> hat >>>>>>>>> >>>>>>>>> to >>>>>>>>> >>>>>>>>> see >>>>>>>>> >>>>>>>>> yet >>>>>>>>> >>>>>>>>> another process enforced that Kees us from getting work done >>>>>>>>> >>>>>>>>> when >>>>>>>>> >>>>>>>>> necessary. >>>>>>>>> >>>>>>>>> -Jake >>>>>>>>> >>>>>>>>> >>>>>>>>> On Dec 19, 2019, at 4:43 PM, Dan Smith < >>>>>>>>> >>>>>>>>> dsm...@pivotal.io >>>>>>>>> >>>>>>>>> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> -1 to (1) and (2). >>>>>>>>> >>>>>>>>> I think merge commits are appropriate in certain >>>>>>>>> >>>>>>>>> circumstances, >>>>>>>>> >>>>>>>>> so I >>>>>>>>> >>>>>>>>> don't >>>>>>>>> >>>>>>>>> think we should make a blanket restriction. In fact I >>>>>>>>> >>>>>>>>> think >>>>>>>>> >>>>>>>>> our >>>>>>>>> >>>>>>>>> release >>>>>>>>> >>>>>>>>> process involves some merges. >>>>>>>>> >>>>>>>>> I think setting standards on what is reasonable to be an >>>>>>>>> >>>>>>>>> individual >>>>>>>>> >>>>>>>>> commit >>>>>>>>> >>>>>>>>> will do a lot more to clean up our history than blocking >>>>>>>>> >>>>>>>>> merges. >>>>>>>>> >>>>>>>>> We'd >>>>>>>>> >>>>>>>>> rather not see commits like "Spotless Apply" in the >>>>>>>>> >>>>>>>>> history, >>>>>>>>> >>>>>>>>> but >>>>>>>>> >>>>>>>>> if >>>>>>>>> >>>>>>>>> reasonably separate and well written commits come in as >>>>>>>>> >>>>>>>>> part >>>>>>>>> >>>>>>>>> of >>>>>>>>> >>>>>>>>> a >>>>>>>>> >>>>>>>>> merge, I >>>>>>>>> >>>>>>>>> think that's fine. >>>>>>>>> >>>>>>>>> -Dan >>>>>>>>> >>>>>>>>> On Thu, Dec 19, 2019 at 4:27 PM Jinmei Liao < >>>>>>>>> >>>>>>>>> jil...@pivotal.io >>>>>>>>> >>>>>>>>> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> +1 >>>>>>>>> >>>>>>>>> On Thu, Dec 19, 2019, 4:05 PM Owen Nichols < >>>>>>>>> >>>>>>>>> onich...@pivotal.io >>>>>>>>> >>>>>>>>> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> I’d like to advance this topic from an informal >>>>>>>>> >>>>>>>>> request/discussion >>>>>>>>> >>>>>>>>> to a >>>>>>>>> >>>>>>>>> discussion of a concrete proposal. >>>>>>>>> >>>>>>>>> To recap, it sounds like there is general agreement >>>>>>>>> >>>>>>>>> that >>>>>>>>> >>>>>>>>> commit >>>>>>>>> >>>>>>>>> history >>>>>>>>> >>>>>>>>> on >>>>>>>>> >>>>>>>>> develop should be linear (no merge commits), and that >>>>>>>>> >>>>>>>>> the >>>>>>>>> >>>>>>>>> biggest >>>>>>>>> >>>>>>>>> obstacle >>>>>>>>> >>>>>>>>> to this is that the PR merge button in GitHub creates a >>>>>>>>> >>>>>>>>> merge >>>>>>>>> >>>>>>>>> commit >>>>>>>>> >>>>>>>>> by >>>>>>>>> >>>>>>>>> default. >>>>>>>>> >>>>>>>>> I propose the following changes: >>>>>>>>> 1) Change GitHub settings to remove the ability to >>>>>>>>> >>>>>>>>> create >>>>>>>>> >>>>>>>>> a >>>>>>>>> >>>>>>>>> merge >>>>>>>>> >>>>>>>>> commit. >>>>>>>>> >>>>>>>>> This will make Squash & Merge the default. >>>>>>>>> >>>>>>>>> 2) Change GitHub settings to require linear history on >>>>>>>>> >>>>>>>>> develop. >>>>>>>>> >>>>>>>>> This >>>>>>>>> >>>>>>>>> prevents merge commits via command-line (not >>>>>>>>> >>>>>>>>> recommended, >>>>>>>>> >>>>>>>>> but >>>>>>>>> >>>>>>>>> wiki >>>>>>>>> >>>>>>>>> still >>>>>>>>> >>>>>>>>> has instructions for merging PRs this way). >>>>>>>>> >>>>>>>>> 3) Update the PR template to change the text "Is your >>>>>>>>> >>>>>>>>> initial >>>>>>>>> >>>>>>>>> contribution >>>>>>>>> >>>>>>>>> a single, squashed commit” to “Is your initial >>>>>>>>> >>>>>>>>> contribution >>>>>>>>> >>>>>>>>> squashed >>>>>>>>> >>>>>>>>> for >>>>>>>>> >>>>>>>>> ease of review (e.g. a single commit, or a >>>>>>>>> >>>>>>>>> ‘refactoring’ >>>>>>>>> >>>>>>>>> commit >>>>>>>>> >>>>>>>>> plus >>>>>>>>> >>>>>>>>> a >>>>>>>>> >>>>>>>>> ‘fix’ commit)" >>>>>>>>> >>>>>>>>> For clarity, I am proposing no-change in the following >>>>>>>>> >>>>>>>>> areas: >>>>>>>>> >>>>>>>>> i) Leave Rebase & Merge as an option for PRs that have >>>>>>>>> >>>>>>>>> been >>>>>>>>> >>>>>>>>> structured to >>>>>>>>> >>>>>>>>> benefit from it (this can make it easier in a bisect to >>>>>>>>> >>>>>>>>> see >>>>>>>>> >>>>>>>>> whether >>>>>>>>> >>>>>>>>> the >>>>>>>>> >>>>>>>>> refactoring or the “fix” broke something). >>>>>>>>> ii) Leave existing wording in the wiki as-is [stating >>>>>>>>> >>>>>>>>> that >>>>>>>>> >>>>>>>>> squashing >>>>>>>>> >>>>>>>>> is >>>>>>>>> >>>>>>>>> preferred]. >>>>>>>>> >>>>>>>>> >>>>>>>>> Please comment via this email thread. >>>>>>>>> -Owen >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> On Dec 16, 2019, at 10:49 AM, Kirk Lund < >>>>>>>>> >>>>>>>>> kl...@apache.org> >>>>>>>>> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> I think it's already resolved Udo ;) >>>>>>>>> >>>>>>>>> Here's the problem, if I fixup a dunit test by >>>>>>>>> >>>>>>>>> removing >>>>>>>>> >>>>>>>>> all >>>>>>>>> >>>>>>>>> uses >>>>>>>>> >>>>>>>>> of >>>>>>>>> >>>>>>>>> "this." >>>>>>>>> >>>>>>>>> and I rename the dunit test, then git doesn't remember >>>>>>>>> >>>>>>>>> that >>>>>>>>> >>>>>>>>> the >>>>>>>>> >>>>>>>>> file >>>>>>>>> >>>>>>>>> is a >>>>>>>>> >>>>>>>>> rename -- it forever afterwards interprets it as a new >>>>>>>>> >>>>>>>>> file >>>>>>>>> >>>>>>>>> that I >>>>>>>>> >>>>>>>>> created >>>>>>>>> >>>>>>>>> if I touch more than 50% of the lines (which "this." >>>>>>>>> >>>>>>>>> can >>>>>>>>> >>>>>>>>> easily >>>>>>>>> >>>>>>>>> do). If >>>>>>>>> >>>>>>>>> we >>>>>>>>> >>>>>>>>> squash two commits: the rename and the cleanup of that >>>>>>>>> >>>>>>>>> dunit >>>>>>>>> >>>>>>>>> test >>>>>>>>> >>>>>>>>> -- >>>>>>>>> >>>>>>>>> then >>>>>>>>> >>>>>>>>> we effectively lose the history of that file and it >>>>>>>>> >>>>>>>>> shows >>>>>>>>> >>>>>>>>> that >>>>>>>>> >>>>>>>>> I >>>>>>>>> >>>>>>>>> created >>>>>>>>> >>>>>>>>> a >>>>>>>>> >>>>>>>>> new file. >>>>>>>>> >>>>>>>>> Also for the record, I've been working on Geode since >>>>>>>>> >>>>>>>>> the >>>>>>>>> >>>>>>>>> beginning >>>>>>>>> >>>>>>>>> and I >>>>>>>>> >>>>>>>>> was never made aware of this change in our process. I >>>>>>>>> >>>>>>>>> never >>>>>>>>> >>>>>>>>> voted >>>>>>>>> >>>>>>>>> on >>>>>>>>> >>>>>>>>> it. >>>>>>>>> >>>>>>>>> I'm not a big fan of changing various details in our >>>>>>>>> >>>>>>>>> process >>>>>>>>> >>>>>>>>> every >>>>>>>>> >>>>>>>>> single >>>>>>>>> >>>>>>>>> week. It's very easy to miss these discussions unless >>>>>>>>> >>>>>>>>> someone >>>>>>>>> >>>>>>>>> points it >>>>>>>>> >>>>>>>>> out >>>>>>>>> >>>>>>>>> to me. >>>>>>>>> >>>>>>>>> On Mon, Dec 16, 2019 at 10:34 AM Udo Kohlmeyer < >>>>>>>>> >>>>>>>>> ukohlme...@pivotal.io> >>>>>>>>> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> I'm not sure what this discussion is about... WE, as >>>>>>>>> >>>>>>>>> a >>>>>>>>> >>>>>>>>> community, >>>>>>>>> >>>>>>>>> have >>>>>>>>> >>>>>>>>> agreed in common practices, in two place no less... >>>>>>>>> >>>>>>>>> 1) Quoting our PR template >>>>>>>>> >>>>>>>>> >>>>>>>>> For all changes: >>>>>>>>> >>>>>>>>> * >>>>>>>>> >>>>>>>>> Is there a JIRA ticket associated with this PR? Is it >>>>>>>>> >>>>>>>>> referenced >>>>>>>>> >>>>>>>>> in >>>>>>>>> >>>>>>>>> the commit message? >>>>>>>>> >>>>>>>>> * >>>>>>>>> >>>>>>>>> Has your PR been rebased against the latest commit >>>>>>>>> >>>>>>>>> within >>>>>>>>> >>>>>>>>> the >>>>>>>>> >>>>>>>>> target >>>>>>>>> >>>>>>>>> branch (typically|develop|)? >>>>>>>>> >>>>>>>>> * >>>>>>>>> >>>>>>>>> ***Is your initial contribution a single, squashed >>>>>>>>> >>>>>>>>> commit?* >>>>>>>>> >>>>>>>>> >>>>>>>>> * >>>>>>>>> >>>>>>>>> Does|gradlew build|run cleanly? >>>>>>>>> >>>>>>>>> * >>>>>>>>> >>>>>>>>> Have you written or updated unit tests to verify your >>>>>>>>> >>>>>>>>> changes? >>>>>>>>> >>>>>>>>> >>>>>>>>> * >>>>>>>>> >>>>>>>>> If adding new dependencies to the code, are these >>>>>>>>> >>>>>>>>> dependencies >>>>>>>>> >>>>>>>>> licensed in a way that is compatible for inclusion >>>>>>>>> >>>>>>>>> underASF >>>>>>>>> >>>>>>>>> 2.0 >>>>>>>>> >>>>>>>>> < >>>>>>>>> >>>>>>>>> http://www.apache.org/legal/resolved.html#category-a >>>>>>>>> >>>>>>>>> ? >>>>>>>>> >>>>>>>>> >>>>>>>>> On our PR template we call out that the initial PR >>>>>>>>> >>>>>>>>> commit >>>>>>>>> >>>>>>>>> should >>>>>>>>> >>>>>>>>> be >>>>>>>>> >>>>>>>>> squashed. >>>>>>>>> >>>>>>>>> 2) >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>> https://cwiki.apache.org/confluence/display/GEODE/Code+contributions >>>>>>>>> >>>>>>>>> < >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>> https://cwiki.apache.org/confluence/display/GEODE/Code+contributions >>>>>>>>> >>>>>>>>> -- See "Accepting a PR Using the Command Line" - >>>>>>>>> >>>>>>>>> Point >>>>>>>>> >>>>>>>>> #3. >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> @Kirk, if each of your commits "stands alone", I >>>>>>>>> >>>>>>>>> commend >>>>>>>>> >>>>>>>>> you >>>>>>>>> >>>>>>>>> on >>>>>>>>> >>>>>>>>> the >>>>>>>>> >>>>>>>>> diligence, but in reality, they should either then be >>>>>>>>> >>>>>>>>> stand >>>>>>>>> >>>>>>>>> alone >>>>>>>>> >>>>>>>>> PR's >>>>>>>>> >>>>>>>>> or just extra work created for yourself. >>>>>>>>> >>>>>>>>> If we want to change the way we have agreed upon we >>>>>>>>> >>>>>>>>> submit/commit/merge >>>>>>>>> >>>>>>>>> changes back into develop... Then this is another >>>>>>>>> >>>>>>>>> discussion >>>>>>>>> >>>>>>>>> thread, >>>>>>>>> >>>>>>>>> until then, I think we should all remind ourselves on >>>>>>>>> >>>>>>>>> our >>>>>>>>> >>>>>>>>> agreed >>>>>>>>> >>>>>>>>> contributions code of conduct. >>>>>>>>> >>>>>>>>> --Udo >>>>>>>>> >>>>>>>>> On 12/16/19 9:59 AM, Nabarun Nag wrote: >>>>>>>>> >>>>>>>>> Kirk, I believe that creating a Pull Request with >>>>>>>>> >>>>>>>>> multiple >>>>>>>>> >>>>>>>>> commits is >>>>>>>>> >>>>>>>>> ok. >>>>>>>>> >>>>>>>>> It's just in the end that when it's being pushed to >>>>>>>>> >>>>>>>>> develop >>>>>>>>> >>>>>>>>> branch, >>>>>>>>> >>>>>>>>> it >>>>>>>>> >>>>>>>>> needs to be squash merged. I believe that is what >>>>>>>>> >>>>>>>>> you >>>>>>>>> >>>>>>>>> have >>>>>>>>> >>>>>>>>> mentioned >>>>>>>>> >>>>>>>>> in >>>>>>>>> >>>>>>>>> the >>>>>>>>> >>>>>>>>> first paragraph, and I am more than happy with that. >>>>>>>>> If you can see in the first screenshot comparison >>>>>>>>> >>>>>>>>> that >>>>>>>>> >>>>>>>>> I >>>>>>>>> >>>>>>>>> had >>>>>>>>> >>>>>>>>> attached >>>>>>>>> >>>>>>>>> in >>>>>>>>> >>>>>>>>> the first email in this thread is what I want to >>>>>>>>> >>>>>>>>> avoid. >>>>>>>>> >>>>>>>>> >>>>>>>>> Thank you for your feedback. >>>>>>>>> >>>>>>>>> Regards >>>>>>>>> Naba >>>>>>>>> >>>>>>>>> >>>>>>>>> On Mon, Dec 16, 2019 at 9:47 AM Kirk Lund < >>>>>>>>> >>>>>>>>> kl...@apache.org> >>>>>>>>> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> Whenever I submit a PR with multiple commits that I >>>>>>>>> >>>>>>>>> intend >>>>>>>>> >>>>>>>>> to >>>>>>>>> >>>>>>>>> rebase >>>>>>>>> >>>>>>>>> to >>>>>>>>> >>>>>>>>> develop, I always try to ensure that each commit >>>>>>>>> >>>>>>>>> stands >>>>>>>>> >>>>>>>>> alone >>>>>>>>> >>>>>>>>> as >>>>>>>>> >>>>>>>>> is >>>>>>>>> >>>>>>>>> (compiles and passes tests). Separating file >>>>>>>>> >>>>>>>>> renames >>>>>>>>> >>>>>>>>> and >>>>>>>>> >>>>>>>>> refactoring >>>>>>>>> >>>>>>>>> from >>>>>>>>> >>>>>>>>> behavior changes into different commits seems very >>>>>>>>> >>>>>>>>> valuable >>>>>>>>> >>>>>>>>> to >>>>>>>>> >>>>>>>>> me, >>>>>>>>> >>>>>>>>> and >>>>>>>>> >>>>>>>>> I've >>>>>>>>> >>>>>>>>> had trouble getting people to review PRs without >>>>>>>>> >>>>>>>>> this >>>>>>>>> >>>>>>>>> separation >>>>>>>>> >>>>>>>>> (but >>>>>>>>> >>>>>>>>> it >>>>>>>>> >>>>>>>>> could be squashed as it's merged to develop). >>>>>>>>> >>>>>>>>> It sounds to me like the real problem is (a) some >>>>>>>>> >>>>>>>>> PRs >>>>>>>>> >>>>>>>>> have >>>>>>>>> >>>>>>>>> multiple >>>>>>>>> >>>>>>>>> commits >>>>>>>>> >>>>>>>>> that don't compile or don't pass tests, and (b) >>>>>>>>> >>>>>>>>> some >>>>>>>>> >>>>>>>>> PRs >>>>>>>>> >>>>>>>>> that >>>>>>>>> >>>>>>>>> should >>>>>>>>> >>>>>>>>> be >>>>>>>>> >>>>>>>>> merged with squash are not (by accident most >>>>>>>>> >>>>>>>>> likely). >>>>>>>>> >>>>>>>>> >>>>>>>>> I can submit multiple PRs instead of one PR with >>>>>>>>> >>>>>>>>> multiple >>>>>>>>> >>>>>>>>> commits. >>>>>>>>> >>>>>>>>> So >>>>>>>>> >>>>>>>>> I'll >>>>>>>>> >>>>>>>>> change my response to -0 if that helps prevent >>>>>>>>> >>>>>>>>> commits >>>>>>>>> >>>>>>>>> to >>>>>>>>> >>>>>>>>> develop >>>>>>>>> >>>>>>>>> that >>>>>>>>> >>>>>>>>> don't compile or pass tests. Without preventing >>>>>>>>> >>>>>>>>> rebase >>>>>>>>> >>>>>>>>> or >>>>>>>>> >>>>>>>>> merge >>>>>>>>> >>>>>>>>> commits >>>>>>>>> >>>>>>>>> from github, I'm not sure how we can really enforce >>>>>>>>> >>>>>>>>> this >>>>>>>>> >>>>>>>>> or >>>>>>>>> >>>>>>>>> prevent >>>>>>>>> >>>>>>>>> (b) >>>>>>>>> >>>>>>>>> above. >>>>>>>>> >>>>>>>>> On Fri, Dec 13, 2019 at 3:38 PM Alexander Murmann < >>>>>>>>> >>>>>>>>> amurm...@pivotal.io> >>>>>>>>> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> I wonder if Kirk's and Naba's statements are >>>>>>>>> >>>>>>>>> necessarily >>>>>>>>> >>>>>>>>> at >>>>>>>>> >>>>>>>>> odds. >>>>>>>>> >>>>>>>>> Make the change easy (warning: this may be hard), >>>>>>>>> >>>>>>>>> then >>>>>>>>> >>>>>>>>> make >>>>>>>>> >>>>>>>>> the >>>>>>>>> >>>>>>>>> easy >>>>>>>>> >>>>>>>>> change." >>>>>>>>> >>>>>>>>> -Kent Beck >>>>>>>>> >>>>>>>>> Following Kent Beck's advise might reasonably >>>>>>>>> >>>>>>>>> split >>>>>>>>> >>>>>>>>> into >>>>>>>>> >>>>>>>>> two >>>>>>>>> >>>>>>>>> commits. >>>>>>>>> >>>>>>>>> One >>>>>>>>> >>>>>>>>> refactor commit and a separate commit that >>>>>>>>> >>>>>>>>> introduces >>>>>>>>> >>>>>>>>> the >>>>>>>>> >>>>>>>>> actual >>>>>>>>> >>>>>>>>> change. >>>>>>>>> >>>>>>>>> They should be individually revertible and might >>>>>>>>> >>>>>>>>> be >>>>>>>>> >>>>>>>>> easier >>>>>>>>> >>>>>>>>> understood >>>>>>>>> >>>>>>>>> if >>>>>>>>> >>>>>>>>> split out. I vividly remember a change on our code >>>>>>>>> >>>>>>>>> base >>>>>>>>> >>>>>>>>> where >>>>>>>>> >>>>>>>>> someone >>>>>>>>> >>>>>>>>> did a >>>>>>>>> >>>>>>>>> huge amount of refactoring that resulted than in >>>>>>>>> >>>>>>>>> one >>>>>>>>> >>>>>>>>> parameter >>>>>>>>> >>>>>>>>> changing >>>>>>>>> >>>>>>>>> in >>>>>>>>> >>>>>>>>> order to get the desired functionality change. If >>>>>>>>> >>>>>>>>> that >>>>>>>>> >>>>>>>>> was >>>>>>>>> >>>>>>>>> in >>>>>>>>> >>>>>>>>> one >>>>>>>>> >>>>>>>>> commit, >>>>>>>>> >>>>>>>>> it would be hard to see the actual change. If >>>>>>>>> >>>>>>>>> split >>>>>>>>> >>>>>>>>> out, >>>>>>>>> >>>>>>>>> it's >>>>>>>>> >>>>>>>>> beautiful >>>>>>>>> >>>>>>>>> and >>>>>>>>> >>>>>>>>> crystal clear. >>>>>>>>> >>>>>>>>> I am unsure how that would be reflected in terms >>>>>>>>> >>>>>>>>> of >>>>>>>>> >>>>>>>>> JIRA >>>>>>>>> >>>>>>>>> ticket >>>>>>>>> >>>>>>>>> references. >>>>>>>>> >>>>>>>>> Usually we assume that if there is a commit with >>>>>>>>> >>>>>>>>> the >>>>>>>>> >>>>>>>>> ticket >>>>>>>>> >>>>>>>>> number, >>>>>>>>> >>>>>>>>> the >>>>>>>>> >>>>>>>>> issue is resolved. Maybe the key here is to create >>>>>>>>> >>>>>>>>> a >>>>>>>>> >>>>>>>>> separate >>>>>>>>> >>>>>>>>> refactoring >>>>>>>>> >>>>>>>>> ticket. >>>>>>>>> >>>>>>>>> Would that allow us to have our cake and eat it >>>>>>>>> >>>>>>>>> too? >>>>>>>>> >>>>>>>>> >>>>>>>>> On Fri, Dec 13, 2019 at 3:16 PM Nabarun Nag < >>>>>>>>> >>>>>>>>> n...@pivotal.io> >>>>>>>>> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> It is to help with bisect operations when things >>>>>>>>> >>>>>>>>> start >>>>>>>>> >>>>>>>>> failing >>>>>>>>> >>>>>>>>> ... >>>>>>>>> >>>>>>>>> helps >>>>>>>>> >>>>>>>>> us >>>>>>>>> >>>>>>>>> it revert and build faster. >>>>>>>>> also with cherry picking features / fixes to >>>>>>>>> >>>>>>>>> previous >>>>>>>>> >>>>>>>>> versions >>>>>>>>> >>>>>>>>> . >>>>>>>>> >>>>>>>>> And keeping the git history clean with no >>>>>>>>> >>>>>>>>> unnecessary >>>>>>>>> >>>>>>>>> “merge >>>>>>>>> >>>>>>>>> commits”. >>>>>>>>> >>>>>>>>> Regards >>>>>>>>> Naba >>>>>>>>> >>>>>>>>> >>>>>>>>> On Fri, Dec 13, 2019 at 2:25 PM Kirk Lund < >>>>>>>>> >>>>>>>>> kl...@apache.org> >>>>>>>>> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> -1 I really like to sometimes have more than 1 >>>>>>>>> >>>>>>>>> commit >>>>>>>>> >>>>>>>>> in >>>>>>>>> >>>>>>>>> a >>>>>>>>> >>>>>>>>> PR >>>>>>>>> >>>>>>>>> and >>>>>>>>> >>>>>>>>> keep >>>>>>>>> >>>>>>>>> them >>>>>>>>> >>>>>>>>> separate when they merge to develop >>>>>>>>> >>>>>>>>> On Tue, Oct 22, 2019 at 5:12 PM Nabarun Nag < >>>>>>>>> >>>>>>>>> n...@pivotal.io> >>>>>>>>> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> Hi Geode Committers, >>>>>>>>> >>>>>>>>> A kind request for using squash commit instead >>>>>>>>> >>>>>>>>> of >>>>>>>>> >>>>>>>>> using >>>>>>>>> >>>>>>>>> merge. >>>>>>>>> >>>>>>>>> This will really help us in our bisect >>>>>>>>> >>>>>>>>> operations >>>>>>>>> >>>>>>>>> when a >>>>>>>>> >>>>>>>>> regression/flakiness in the product is >>>>>>>>> >>>>>>>>> introduced. >>>>>>>>> >>>>>>>>> We >>>>>>>>> >>>>>>>>> can >>>>>>>>> >>>>>>>>> automate >>>>>>>>> >>>>>>>>> and >>>>>>>>> >>>>>>>>> go >>>>>>>>> >>>>>>>>> through fewer commits faster, avoiding commits >>>>>>>>> >>>>>>>>> like >>>>>>>>> >>>>>>>>> "spotless >>>>>>>>> >>>>>>>>> fix" >>>>>>>>> >>>>>>>>> and >>>>>>>>> >>>>>>>>> "re-trigger precheck-in" or other minor commits >>>>>>>>> >>>>>>>>> in >>>>>>>>> >>>>>>>>> the >>>>>>>>> >>>>>>>>> merged >>>>>>>>> >>>>>>>>> branch. >>>>>>>>> >>>>>>>>> Also, please use the commit format : (helps us >>>>>>>>> >>>>>>>>> to >>>>>>>>> >>>>>>>>> know >>>>>>>>> >>>>>>>>> who >>>>>>>>> >>>>>>>>> worked >>>>>>>>> >>>>>>>>> on >>>>>>>>> >>>>>>>>> it, >>>>>>>>> >>>>>>>>> what is the history) >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> * GEODE-xxxx: <brief intro > >>>>>>>>> * explanation line 1 >>>>>>>>> >>>>>>>>> * >>>>>>>>> >>>>>>>>> explanation >>>>>>>>> >>>>>>>>> line >>>>>>>>> >>>>>>>>> 2* >>>>>>>>> >>>>>>>>> This is not a rule or anything, but a request >>>>>>>>> >>>>>>>>> to >>>>>>>>> >>>>>>>>> help >>>>>>>>> >>>>>>>>> out >>>>>>>>> >>>>>>>>> your >>>>>>>>> >>>>>>>>> fellow >>>>>>>>> >>>>>>>>> committers in quickly detecting a problem. >>>>>>>>> >>>>>>>>> For inspiration, we can look into Apache Kafka >>>>>>>>> >>>>>>>>> / >>>>>>>>> >>>>>>>>> Spark >>>>>>>>> >>>>>>>>> where >>>>>>>>> >>>>>>>>> they >>>>>>>>> >>>>>>>>> have >>>>>>>>> >>>>>>>>> a >>>>>>>>> >>>>>>>>> complete linear graph for their main branch >>>>>>>>> >>>>>>>>> HEAD >>>>>>>>> >>>>>>>>> [see >>>>>>>>> >>>>>>>>> attachment] >>>>>>>>> >>>>>>>>> Regards >>>>>>>>> Naba. >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> -- >>>>>>>>> >>>>>>>>> Ju@N >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> -- >>>>>>>>> Cheers >>>>>>>>> >>>>>>>>> Jinmei >>>>>>>> >>>>>>>> >>>>>> >>>>>> >>>>> >>>> >>> >>
