Those comments may or may not be useful anymore. What they describe may have been superseded by a subsequent commit.
On Fri, May 15, 2015 at 3:12 PM, Daan Hoogland <daan.hoogl...@gmail.com> wrote: > those comments will then have to be squashed s well, to this i put a -1. If > those comments where usefull at review-time they will be usefull in the > future as well. > > Op vr 15 mei 2015 om 19:29 schreef Marcus <shadow...@gmail.com>: > > > 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.hoogl...@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 < > run...@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 > > > > > > -- *Mike Tutkowski* *Senior CloudStack Developer, SolidFire Inc.* e: mike.tutkow...@solidfire.com o: 303.746.7302 Advancing the way the world uses the cloud <http://solidfire.com/solution/overview/?video=play>*™*
