cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chip Childers <chip.child...@sungard.com>
Subject Re: [Review Request] Re-enabling baremetal on master
Date Fri, 21 Jun 2013 02:19:06 GMT
Nice!  I'm glad the feature has the benefit of tests now.  Thanks for
doing this Sheng!

David - are you comfortable with this, and will you now +1 the feature?

On Thu, Jun 20, 2013 at 9:55 PM, Sheng Yang <sheng@yasker.org> wrote:
> Hi,
>
> I've updated baremetal-4.2 branch, added integration test for some of
> baremetal related APIs, also fixed a bunch of baremetal API issues exposed
> by the testing.
>
> Thanks!
>
> --Sheng
>
>
>
>
> On Wed, Jun 19, 2013 at 11:41 AM, Chip Childers
> <chip.childers@sungard.com>wrote:
>
>> On Wed, Jun 19, 2013 at 11:36:11AM -0700, Sheng Yang wrote:
>> > On Wed, Jun 19, 2013 at 11:24 AM, Chip Childers
>> > <chip.childers@sungard.com>wrote:
>> >
>> > > On Wed, Jun 19, 2013 at 11:00:33AM -0700, Sheng Yang wrote:
>> > > > Hi,
>> > > >
>> > > > I've created the https://reviews.apache.org/r/11977/  for review.
>> The
>> > > > branch re-enabled the baremetal for master. And all major bugs are
>> > > cleaned.
>> > > >
>> > > > https://issues.apache.org/jira/browse/CLOUDSTACK-1610
>> > > > https://issues.apache.org/jira/browse/CLOUDSTACK-1618
>> > > > https://issues.apache.org/jira/browse/CLOUDSTACK-1614
>> > > > https://issues.apache.org/jira/browse/CLOUDSTACK-1440
>> > > >
>> > > > In fact it's not a feature merge, because the code is already in
>> MASTER
>> > > > ready. We just disable it due to stability problem of 4.1 release.
>> Now
>> > > I've
>> > > > tried to enable it, and the changeset is very small, mostly just
>> revert
>> > > the
>> > > > old disabling baremetal codes, and fix some issues with introducing
>> other
>> > > > new features. Here is the summary:
>> > >
>> > > [snip]
>> > >
>> > > So David's standing veto was because of this comment (from him):
>> > >
>> > > "Baremetal seems to be suffering from a significant lack of unit tests
>> > > and integration tests for marvin to consume. Let's get those in place
>> > > before we consider re-enabling this."
>> > >
>> > > If I remember correctly, the reason that master has the code in it, is
>> > > specifically because we decided that disabling the feature was easier
>> to
>> > > honor the veto than reverting all of the changes.
>> > >
>> > > That being said, have we addressed the original veto's concerns?
>> > >
>> >
>> > Not yet. I didn't realize it's vetoed due to this. Let me see what can I
>> do
>> > about it.
>>
>> Awesome.  Thanks Sheng!
>>
>> >
>> > In fact the above bugs cannot be detected for unit test or marvin test(I
>> > even not sure if they're valid bugs or not, but at that time Frank is on
>> > vacation and nobody took a look at these then decided disable the
>> feature,
>> > and after I re-enabled them, everything works fine for me).
>>
>> Yeah, I think that the bugs were just in need of triage.  The bugs
>> themselves weren't the major issue (although they were concerning), as
>> much as test coverage at either (or both) unit or integration levels.
>>
>> -chip
>>

Mime
View raw message