trafodion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dave Birdsall <dave.birds...@esgyn.com>
Subject RE: Squash commits
Date Mon, 18 Jul 2016 19:08:09 GMT
+1

I gave the "squash commit" a try, and noticed the issues Hans' raises. I
found it not to my taste.

Another issue I noticed is the SHA for the commit doesn't go into the
original pull request which might make later cherry-picking a bit more of a
hassle.

Dave

-----Original Message-----
From: Hans Zeller [mailto:hans.zeller@esgyn.com]
Sent: Monday, July 18, 2016 12:04 PM
To: dev <dev@trafodion.incubator.apache.org>
Subject: Re: Squash commits

Hi,

Sorry to chime in late. I would like to propose that we do NOT squash the
commits in a PR. Instead, we could ask contributors to squash, if their
changes consist of too many commits.

Here are my reasons:

   1. Squashing the commits takes away the contributor's commit and makes
   it looks as if the committer implemented the change. IMHO that is
   undesirable, I always liked the fact that our history shows the
   contributors' work very clearly.
   2. We should not make the commit workflow more complicated. I would
   prefer that contributors do the work, and create a reasonable number of
   commits with clearly defined features. That also makes the review process
   easier.
   3. A more minor point: Commits that resolve merge conflicts only should
   start with "Merge". Otherwise (the commit contains actual features not in
   another commit), the title should not start with "Merge".

Thanks,

Hans

On Fri, Jul 15, 2016 at 9:29 AM, Steve Varnau <steve.varnau@esgyn.com>
wrote:

> I don't think there is a guaranteed way in git.  It relies on the
> committer putting the right info in the commit message.
> I think the most important thing is the JIRA ID.
>
> The message might also contain the commit IDs of the squashed branch
> commits, but that is harder to recognize, though a search in the log
> may find it.
>
> Also if the committer puts in the magic phrase that causes the Apache
> automation to close the PR, then the JIRA may also get updated.
> Again, relying on care taken by the committer.  I'm not a big fan of
> squashed commits because all of this. But it is matter of preference,
> and they definitely are useful in certain cases.
>
> Developers & committers, please, please put the JIRA ID in brackets at
> the beginning of the line in the PR title and in the merged commit
> message. And also at least a couple words that describe the change.
> "fix for JIRA 20112"
> does not help me a whole lot.
> "[TRAFODION-20112] compiler NULL fix"  is not real specific, but gives
> a clue and makes sure that the JIRA gets updated with the right PR and
> commit info!
>
> Reviewers, please remind developers to update the PR title to the
> right format. I see Selva asked someone today. Gold star.
>
> --Steve
>
> > -----Original Message-----
> > From: Dave Birdsall [mailto:dave.birdsall@esgyn.com]
> > Sent: Thursday, July 14, 2016 2:25 PM
> > To: dev@trafodion.incubator.apache.org
> > Subject: Squash commits
> >
> > Hi,
> >
> >
> >
> > One thing I’m noticing about squash commits is that the original
> > pull request does not automatically get updated with the SHA of the
> > actual merge.
> >
> >
> >
> > If one needed to cherry-pick the changes to another branch, is there
> > an easy way to discover the proper SHA?
> >
> >
> >
> > Thanks,
> >
> >
> >
> > Dave
>

Mime
View raw message