Once a PR is reviewed and merged, the remainder of its lifecycle could include a revert, a cherry-pick to another branch, or just browsing the git history.
In general it’s most convenient for all of the above cases if 1 JIRA == 1 commit on develop. As a set-it-and-forget-it default, Squash and Merge takes care of this for you. However, there are two other options in GitHub, and I think what most people really dislike is the “Create a merge commit” option. If you have carefully prepared your PR in a way that "Rebase and Merge” makes sense, I don’t necessarily see a problem with that. Just keep in mind the cherry-pick case — if your “fix” commit depends on your carefully-separated “refactor” commit, maybe squash is just better so the whole fix can be cherry-picked as a single unit in the future. If the commits are relatively independent, maybe delivering as multiple PRs would better express that. If the problem is that our PR process is too “heavy”, and you’re trying to deliver multiple commits via one PR vehicle just to cut down on overhead, perhaps this merits further community discussion. Is getting reviewers the biggest difficulty, or waiting for PR checks to pass, or is it just too much busywork creating a JIRA, etc, for each change? > On 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. >>>>> >>>>> >>>>> >>>> >>> >>
