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 > > > > > >> > > > > > >> > > > > > > > > > > > > > > > > > > > >
