Sateesh and Rajesh, It seems you were the only guys who +1 the squash idea. Could you please share with us what benefits you think squashing commits will bring?
I wil give you the simplest example that could come to my mind to encourage no squash: * I open a Java class with 5 thousand lines. The first thing I do is format the code and commit the change. * I go back to the class and apply the fix, let’s say, a 3 lines change, then I commit the change again. Now, think about the PR. It will contain 2 commits: 1 with the formatting changes only; and a second commit with 3 lines change. Would you like to see it quashed and all messed up? It would be very difficult to review. That’s just a simple example. Cheers, Wilder > On 02 Jul 2015, at 07:22, Rajesh Battala <rajesh.batt...@citrix.com> wrote: > > +1 for squashing commit > > -----Original Message----- > From: John Burwell [mailto:john.burw...@shapeblue.com] > Sent: Thursday, July 2, 2015 12:14 AM > To: dev@cloudstack.apache.org > Subject: Re: [PROPOSAL] Commit to master through PR only > > All, > > I think we should stick to 2 votes per PR. Defining types of PRs becomes > difficult bordering on the arbitrary — adding a process complexity and the > potential to start debating if a particular PR is one type or another. > > I agree regarding the fast forward, and feel that all PRs should squashed > down to one commit. Ultimately, intermediate commits that seem informative > in a feature branch become noise in a history as large as CloudStack’s. > > To enforce the policy and ensure that PRs are merged in an orderly and > correct manner (i.e. one at time), I think we should consider adopting a tool > such as bors [1] to verify that the merge passes all tests and then performs > the merge. It would some minor modification to require two votes, but I doubt > that would take much effort to implement. If there is interest, I would > happy to make those changes for the project. > > Thanks, > -John > > [1]: https://github.com/graydon/bors > > --- > John Burwell (@john_burwell) > VP of Software Engineering, ShapeBlue > (571) 403-2411 | +44 20 3603 0542 > http://www.shapeblue.com > > > >> On Jul 1, 2015, at 1:48 PM, Rohit Yadav <rohit.ya...@shapeblue.com> wrote: >> >> Hi, >> >>> On 25-Jun-2015, at 4:38 pm, Sebastien Goasguen <run...@gmail.com> wrote: >>> >>> A few of us are in Amsterdam at DevOps days. We are chatting about release >>> management procedure. >>> Remi is working on a set of principles that he will put on the wiki to >>> start a [DISCUSS]. >>> >>> However to get started on the right track. I would like to propose the >>> following easy step: >>> >>> Starting Monday June 29th (next monday): >>> >>> - Only commit through PR will land on master (after a minimum of 2 LGTM and >>> green Travis results) >>> - Direct commit will be reverted >>> - Any committer can merge the PR. >> >> +1 >> >> I’ve been trying to help close PRs, it was difficult at first but then I >> found some tooling to help me do that. I think it’s certainly do-able >> without investing a lot of effort to do it, perhaps can done everyday or >> every few days in a week. >> >> Some suggestions and comments to improve PR reviewing/merging: >> >> - Let's merge the PR commits in a fast forward way instead of doing a branch >> merge that introduces frivolous merge commits. This is one approach to do >> quickly and painlessly: >> >> http://blog.remibergsma.com/2015/05/24/accepting-pull-requests-the-easy-way/ >> >> - Let’s try to send PR around on one issue or one broad issue, or against a >> JIRA ticket; but avoid unrelated sub-systems etc >> >> - If there are not many changes (say less than 100-200 lines were changed), >> let's have the changes melded into one commit. This can be done either by >> the PR author or by the committer. The immediate benefit is that all the >> changes will be much easy to port across other branches, easy to view and >> follow git-log, and easy to revert-able. >> >> - Certain PRs that are typographical fixes, doc fixes and tooling related >> fixes - so let’s review and merge them if we’ve at least one green review >> (“LGTM”), though changes to CloudStack mgmt server, agent and plugins >> codebase IMO should have at least 2 green reviews (“LGTM”). >> >>> Goal being to start having a new practice -everything through PR for >>> everyone- which is an easy way to gate our own commits building up to a PR. >>> >>> There is no tooling involved, just human agreement. >>> >>> cheers, >> >> Regards, >> Rohit Yadav >> Software Architect, ShapeBlue >> M. +91 88 262 30892 | rohit.ya...@shapeblue.com >> Blog: bhaisaab.org | Twitter: @_bhaisaab >> >> >> >> Find out more about ShapeBlue and our range of CloudStack related services >> >> IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//> >> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/> >> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/> >> CloudStack Software >> Engineering<http://shapeblue.com/cloudstack-software-engineering/> >> CloudStack Infrastructure >> Support<http://shapeblue.com/cloudstack-infrastructure-support/> >> CloudStack Bootcamp Training >> Courses<http://shapeblue.com/cloudstack-training/> >> >> This email and any attachments to it may be confidential and are intended >> solely for the use of the individual to whom it is addressed. Any views or >> opinions expressed are solely those of the author and do not necessarily >> represent those of Shape Blue Ltd or related companies. If you are not the >> intended recipient of this email, you must neither take any action based >> upon its contents, nor copy or show it to anyone. Please contact the sender >> if you believe you have received this email in error. Shape Blue Ltd is a >> company incorporated in England & Wales. ShapeBlue Services India LLP is a >> company incorporated in India and is operated under license from Shape Blue >> Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil >> and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a >> company registered by The Republic of South Africa and is traded under >> license from Shape Blue Ltd. ShapeBlue is a registered trademark. > > Find out more about ShapeBlue and our range of CloudStack related services > > IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//> > CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/> > CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/> > CloudStack Software > Engineering<http://shapeblue.com/cloudstack-software-engineering/> > CloudStack Infrastructure > Support<http://shapeblue.com/cloudstack-infrastructure-support/> > CloudStack Bootcamp Training > Courses<http://shapeblue.com/cloudstack-training/> > > This email and any attachments to it may be confidential and are intended > solely for the use of the individual to whom it is addressed. Any views or > opinions expressed are solely those of the author and do not necessarily > represent those of Shape Blue Ltd or related companies. If you are not the > intended recipient of this email, you must neither take any action based upon > its contents, nor copy or show it to anyone. Please contact the sender if you > believe you have received this email in error. Shape Blue Ltd is a company > incorporated in England & Wales. ShapeBlue Services India LLP is a company > incorporated in India and is operated under license from Shape Blue Ltd. > Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is > operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company > registered by The Republic of South Africa and is traded under license from > Shape Blue Ltd. ShapeBlue is a registered trademark.
