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