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

Reply via email to