cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Marcus <shadow...@gmail.com>
Subject Re: Preparing for 4.6
Date Fri, 15 May 2015 17:29:43 GMT
I'm not sure it is any different. Either you have a giant block of code
that represents a bunch of little commits, or a giant block that is one
commit. We don't want to merge little chunks to master that don't fully
implement the feature.

To the extent that features and goals can be split up, yes, we don't want
those features squashed together, or even submitted together, squashed or
not. An example of this is in combining formatting/syntax fixes with
functional changes, I have tried to review such pull requests and it is
very difficult to see what is going on in a 1k line request when 2/3 of the
changes are just reformatting noise.

Ideally the code is reviewed at the commit level as each small commit goes
from the developer's workstation to the dev branch, but I don't see a way
around reviewing the same amount of code in a pull request that represents
multiple small commits vs one squashed commit. You do get more visibility
into the comments, though.

I suppose a way to get both would be to branch master, do a pull request
from your dev branch to that branch, at which point it is reviewed, then
squash merge that back into master.
On May 15, 2015 8:55 AM, "Daan Hoogland" <daan.hoogland@gmail.com> wrote:

> Sebastien, I don't think commits in a big feature should be squashed. It
> hinders review if to big chunks are submitted at once.
>
> Op vr 15 mei 2015 om 11:27 schreef Sebastien Goasguen <runseb@gmail.com>:
>
> > Folks,
> >
> > As we prepare to try a new process for 4.6 release it would be nice to
> > start paying attention to master.
> >
> > - Good commit messages
> > - Reference to a JIRA bug
> > - Squashing commits ( cc/ wilder :))
> >
> > While you can apply patches directly, good practice is to apply the patch
> > to a branch and then merge that branch into master.
> >
> > Before we start enforcing some of these things more strongly, please keep
> > it in mind.
> >
> > thanks,
> >
> > -sebastien
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message