On Fri, Feb 15, 2013 at 11:03:27AM -0800, Edison Su wrote: > > > > -----Original Message----- > > From: Hugo Trippaers [mailto:htrippa...@schubergphilis.com] > > Sent: Friday, February 15, 2013 6:39 AM > > To: cloudstack-dev@incubator.apache.org > > Cc: John Burwell <jburw...@basho.com> (jburw...@basho.com); Mike > > Tutkowski (mike.tutkow...@solidfire.com) > > Subject: RE: [MERGE]Storage refactor branch > > > > Hey Edison, > > > > Thanks for the update > > > > > -----Original Message----- > > > From: Edison Su [mailto:edison...@citrix.com] > > > Sent: Friday, February 15, 2013 12:13 AM > > > To: cloudstack-dev@incubator.apache.org > > > Cc: John Burwell <jburw...@basho.com> (jburw...@basho.com); Mike > > > Tutkowski (mike.tutkow...@solidfire.com) > > > Subject: [MERGE]Storage refactor branch > > > > > > Hi All, > > > Here is the update from storage refactor branch since last week: > > > > > > 1. clean/hook up snapshot into new storage framework, separate > > > taking > > > snapshot and backup snapshot. Add a snapshotstrategy interface for > > > people want to change or replace how snapshot is processed in > > > cloudstack. Current snapshot code is little bit hacky, welcome to replace > > > it > > for your need. > > > Since last week I created storage_refactor branch, seems no > > > objection for what I am doing, so I want to merge the branch into > > > master, by end of this week, due to the following reasons: > > > Maintain such big patch(more than 20K) outside of master, > > > touch all the storage code, is not an easy task. > > > After merge into master, other developers who developing > > > storage features can work on master directly, thus I don't need to > > > rebase the branch regularly. > > > About test, I only tested basic features, like, stop/start vm, > > > attach/detach volumes, take snapshots in devcloud. For sure, I'll > > > break something. The more test from other people, will help to me to > > > stabilize the code. About unit test, I have some unit test and > > > integration test for devcloud, but both of them needs database and > > > devcloud environment, so they are disabled by default and unit test > > > themselves are broken also. I'll write more tests during the > > > stabilization. > > > > I understand the need to merge into master, it is a serious pain to keep > > updating the branch with that latest state of master. However this testing > > Thanks for understand the painful of rebaseing such big patch. There is > another reason I want to merge this branch into master ASAP: all the new > storage features are depended this branch: like zone-wide storage, make nfs > secondary storage as optional, re-write s3/swift integration etc. Let's get > it merged, adding more awesome features:) > > > concerns me a lot, we all agreed that we would focus on automated testing > > both on the unittest level and on the functional level. This is the first > > merge > > request since we had the last discussion on testing (related to merging the > > javelin and other branches). The clear consensus was that commits would > > have to be accompanied by unittests (at least for the changed pieces of > > code) > > and preferable some automated functional test. > > > > Considering that, I think that a merge is not the right this to do at the > > moment. Instead of merging into master and then fixing stuff, we should > > focus on adding testing to this branch so we can merge a well-tested unit > > later. Notice my use of 'we' here, we all said focus on testing, so we all > > should > > help out. Unfortunately I'm pretty busy with packaging and the $dayjob, but > > if you have something I can help you with regarding setting up unittests > > feel > > free to let me know. You already mentioned that you have some, but the > > databases need to be mocked? > > > > It would be great if you could spend some time to details what tests you > > have in place, so we can fix those. With the help of the cobertura report we > > can figure out where the risks are. Prassana and myself are also working on > > the automated test framework, so if you have some marvin tests, we can > > help to automate them. > > My branch is not a feature branch, while other features are depended on it. I > didn't add any new feature on the branch, all the existing marvin automated > tests should work. Instead of testing and fixing on my branch then merge, is > it better to test and fix on master after the merge, using existing marvin > test?
IMO, it's not ever good to intentionally to break master.
