Hi Mark, I would like to limit the scope of the discussion to pushing PRs to develop as a squash and merge operation and not just merge. This is to keep the history linear and clean like other projects which will help with regression and cherry-pick (reference: first screenshot in the email chain)
How many commits should be there in a PR can be placed in a different discussion thread. Regards Naba On Mon, Dec 16, 2019 at 10:02 AM Mark Hanson <mhan...@pivotal.io> wrote: > 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. > >>>>> > >>>>> > >>>>> > >>>> > >>> > >> > >
