cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Nalley <da...@gnsa.us>
Subject Re: [Review Request] Re-enabling baremetal on master
Date Fri, 21 Jun 2013 02:22:15 GMT
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