This was the part I was referring to, from the existing PR template: > 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)"
Either of these, while I guess not strictly required(?), is a change from what SOP has been for native client. We rebase onto the current develop when submitting a PR, but don't pre-squash commits in any meaningful way, because it's unnecessary. The rest is fine - changing the default option to Squash & Merge is the right thing to do IMO. Thanks, Blake On Fri, Dec 20, 2019 at 12:00 PM Nabarun Nag <n...@pivotal.io> wrote: > Just to reiterate. Nothing changes in the workflow of a developer. It's > just in the end, when all the reviews are done and all the tests are > passing, then the button to click in the Github web UI is "Squash and > Merge". That's all. > > Regards > Naba > > > > On Fri, Dec 20, 2019 at 11:55 AM Blake Bender <bben...@pivotal.io> wrote: > > > Very well, then, I'll abide by the group consensus but am on the record > as: > > * strongly opposed to multi-commit PRs, because I believe it clutters > > history, and > > * also not a big fan of forcing devs to rebase and squash prior to > > submitting a PR. IMO this is busy work, and *may* in some small minority > > of cases hide information that would be useful to reviewers. > > > > Thanks, > > > > Blake > > > > > > 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 > > > >> > > > > > > > > >
