Hey hey, On Sep 23, 2014, at 11:50 PM, David Nalley <da...@gnsa.us> wrote: > On Tue, Sep 23, 2014 at 4:44 PM, Rohit Yadav <rohit.ya...@shapeblue.com> > wrote: >> Hi David, >> >> On 23-Sep-2014, at 8:31 pm, David Nalley <da...@gnsa.us> wrote: >>> Where was the merge request for this huge merge to master? (it was at >>> 50 commit emails, when it stopped sending, ) >>> We have passed feature freeze for 4.5.0, so I am confused as why this >>> was merged. Is there a reason not to revert all of this? >> >> This was the request: https://github.com/apache/cloudstack/pull/16 >> And JIRA: https://issues.apache.org/jira/browse/CLOUDSTACK-7143 >> We all get emails from Github PR (just re-checked) so you may find them on >> the ML. > > Yes, GH PR is exactly like the Review Board emails in this particular > aspect. My question is why is this merged into master rather than a > feature branch, and why no [MERGE] email as per: > https://cwiki.apache.org/confluence/display/CLOUDSTACK/Branch+Merge+Expectations
Hmm, I hadn’t seen that before; don’t think it’s linked from the “how to contribute” stuff. Sorry. Or is this something a committer is to do? Can we change the rules? Because I’d guess the [MERGE] e-mail serves exactly the same role as a pull request: widely announcing a branch is ready to merge in and providing a trigger point for discussion. It’s also interesting to consider whether you need a feature branch at all in the local repository for branches that were first developed (and tested) remotely. After all the only remaining thing you will do to that branch is to merge it in. The original feature branch is https://github.com/schubergphilis/CloudStack/tree/feature/systemvm-refactor which has existed for a few months and was discussed here a few times (well I sent e-mail about it, there wasn’t much discussion…), with https://github.com/schubergphilis/CloudStack/tree/feature/systemvm-refactor-for-upstream as the re-re-rebased version of all that. This is all github standard (best?) practice, to the point that you cannot even create pull requests that result in the creation of new branches. I.e. if you look at https://github.com/schubergphilis/cloudstack/compare/feature/systemvm-refactor-for-upstream and you click the “edit” in the top row, you can’t even suggest merging to a new branch, there’s only existing ones listed. Of course the committer doing the actual upstreaming can make a different choice, but, why would you? As for whether to take this for 4.5.0, of course the risk is not 0, but its pretty darn low. I also think by typical community rules just about none of the commits in the pull request are a new feature. Longer term the point of these changes is a quality increase by being just slightly more precise and deliberate in a few places. If you think they don’t accomplish that you should definitely pull it out, but then I’d like to hear what’s wrong! If you pull it out, please do consider something more limited like https://github.com/schubergphilis/cloudstack/commit/74c381540ebaf940bbbf471b580303488aa9c5b4 which avoids putting master version of the systemvm code into branch builds. I would also _really_ like to see a virtualbox systemvm.box out of the official 4.5.0 build so that we can easily use our systemvm component tests for regression testing the changes we’ll have for 4.5.1/4.6.0. If you back this out it’ll be annoying bit twiddling to make one manually. cheers! Leo
