I would strongly prefer smaller as small a commit as possible. And as large as necessary. I am less partial when it comes to PRs sizes. Sometimes depending on what is done in a PR, I don’t think it makes sense to issue a blanket statement that all PRs are one commit. I think there is a strong reason to make them one commit, but one size does not fit all. A great example is a refactor and a bug fix in one PR.
Thanks, Mark > 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. >>>>> >>>>> >>>>> >>>> >>> >>
