cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Leo Simons <LSim...@schubergphilis.com>
Subject Re: [01/50] git commit: updated refs/heads/master to 1290e10
Date Wed, 24 Sep 2014 06:20:57 GMT
Hey hey,

On Sep 23, 2014, at 11:50 PM, David Nalley <david@gnsa.us> wrote:
> On Tue, Sep 23, 2014 at 4:44 PM, Rohit Yadav <rohit.yadav@shapeblue.com> wrote:
>> Hi David,
>> 
>> On 23-Sep-2014, at 8:31 pm, David Nalley <david@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


Mime
View raw message