cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sheng Yang <sh...@yasker.org>
Subject Re: [Review Request] Re-enabling baremetal on master
Date Fri, 21 Jun 2013 16:45:39 GMT
Thank you all!

I would commit the change to MASTER soon.

--Sheng


On Thu, Jun 20, 2013 at 7:22 PM, David Nalley <david@gnsa.us> wrote:

> Yes
> Happy to +1. Sheng, thanks for stepping up and getting this done.
>
> --David
> On Jun 20, 2013 7:19 PM, "Chip Childers" <chip.childers@sungard.com>
> wrote:
>
> > 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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message