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 >>
