Yes Happy to +1. Sheng, thanks for stepping up and getting this done. --David On Jun 20, 2013 7:19 PM, "Chip Childers" <chip.child...@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 <sh...@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.child...@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.child...@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 > >> >
