[Hawkular-dev] [process] tickets and PRs

Lukas Krejci lkrejci at redhat.com
Thu May 28 08:03:33 EDT 2015


My main objection to this is that the contributor cannot verify their commits 
made it to the repository, because you destroy that information by squashing.

History is easily linearized using git log facilities (--no-merges or --merges 
in particular).

To figure what jira/PR a commit belongs to you can:

git log <COMMIT_HASH>..master --ancestry-path --merges

I.e. you can get the SAME view of history like if you squashed every PR prior 
to merging by doing this:

git log --merges

while at the same time you retain all the information about the evolution of 
the PRs + the contributors can be sure their code is in, because the commit 
hashes still match.

It is also possible to effectively bisect. If you squash a PR consisting of 
5000 lines changed and you bisect a hard-to-debug problem, all you know is 
that somewhere in those 5000 lines there's a problem.

If on the other hand you keep the individual commits of the PR you can bisect 
directly to the arguably smaller commit that introduced the problem and find 
out the reasons for the breakage in a more focused context.

On Thursday, May 28, 2015 07:09:27 Lucas Ponce wrote:
> Hi,
> 
> My suggestion of the use of squash for some scenarios comes from our
> experience with GateIn/Portal:
> 
> https://github.com/gatein/gatein-portal/commits/master
> 
> I'm not talking about granularity.
> 
> I'm +1 to have a PR with non-squash for a review, but once consolidated, a
> squash can help to have a better organized history (i.e. marked with a
> JIRA/Bugzilla related to the task involved).
> 
> And I admit that there are situations where squashing has not sense.
> 
> But the project is growing and it's good to have some "grouping" with some
> logical background about what feature/fix/patch is related.
> 
> Just my 2 cents.
> 
> ----- Original Message -----
> 
> > From: "Lukas Krejci" <lkrejci at redhat.com>
> > To: "Discussions around Hawkular development"
> > <hawkular-dev at lists.jboss.org> Sent: Thursday, May 28, 2015 12:58:50 PM
> > Subject: Re: [Hawkular-dev] [process] tickets and PRs
> > 
> > On Thursday, May 28, 2015 12:50:55 Lukas Krejci wrote:
> > > On Wednesday, May 27, 2015 12:46:29 Juraci Paixão Kröhling wrote:
> > > > On 05/27/2015 12:41 PM, Lucas Ponce wrote:
> > > > > I also propose to squash commits when it has sense.
> > > > 
> > > > +1 , having "one commit per jira" helps a lot in debugging :-)
> > 
> > btw. if you want to figure out how a certain commit got merged into the
> > master, you can use:
> > 
> > git log <COMMIT_HASH>..master --ancestry-path --merges
> > 
> > or
> > 
> > https://github.com/mhagger/git-when-merged
> > 
> > > -1 on squashing all PRs (if that is what you meant).
> > > 
> > > IMHO squashes are a nice feature to get rid of "garbage" commits PRIOR
> > > to
> > > pushing those commits upstream but SHOULD NOT be used to modify any
> > > commits
> > > that exists also remotely.
> > > 
> > > There are at least 4 reasons against squashing, IMHO:
> > > 
> > > 1) possibility of messing things up like Mazz described.
> > > 2) divergence of histories in different repos (git branch|tag --contains
> > > ceases to work)
> > > 3) squashing a large PR renders bisecting useless
> > > 4) it makes it impossible to "follow" a history of a feature development
> > > 
> > > So I agree that it makes sense to squash certain commits, but only
> > > before
> > > they are merged into the official repos.
> > > 
> > > Ideally I think squash should only be used on local commits. But I do
> > > recognize that one may just push stuff to their personal forks that they
> > > later on would like to modify the history of (i.e. you're working on
> > > something and either push quickly commits you didn't "clean out" locally
> > > or
> > > change your mind later about something and don't want to "pollute"
> > > upstream
> > > with commits going back and forth on the same lines of code).
> > > 
> > > So IMHO a) the number of commits for a jira should be proportional to
> > > the
> > > size of the change and b) the commit history should be finalized before
> > > merging the PR and retained using the merge of the PR.
> > > 
> > > > - Juca.
> > > > 
> > > > _______________________________________________
> > > > hawkular-dev mailing list
> > > > hawkular-dev at lists.jboss.org
> > > > https://lists.jboss.org/mailman/listinfo/hawkular-dev
> > > 
> > > _______________________________________________
> > > hawkular-dev mailing list
> > > hawkular-dev at lists.jboss.org
> > > https://lists.jboss.org/mailman/listinfo/hawkular-dev
> > 
> > _______________________________________________
> > hawkular-dev mailing list
> > hawkular-dev at lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/hawkular-dev
> 
> _______________________________________________
> hawkular-dev mailing list
> hawkular-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hawkular-dev




More information about the hawkular-dev mailing list