Take a look at revision 3 of my changes here: https://reviews.apache.org/r/14522/diff/3/#index_header
The changes I made were due to discussion in the reviews. It should be simpler, cleaner and more efficient logic than using comparators. -- Chris Suich chris.su...@netapp.com<mailto:chris.su...@netapp.com> NetApp Software Engineer Data Center Platforms – Cloud Solutions Citrix, Cisco & Red Hat On Oct 23, 2013, at 6:25 PM, Darren Shepherd <darren.s.sheph...@gmail.com<mailto:darren.s.sheph...@gmail.com>> wrote: Sorry, I don't follow your question. I have time today. What would you like me to do? As it stands right now, what is on master, I'm not aware of any issues. Darren On Wed, Oct 23, 2013 at 3:22 PM, SuichII, Christopher <chris.su...@netapp.com<mailto:chris.su...@netapp.com>> wrote: Darren, Would you be able to look into copy the logic back into your refactoring today or tomorrow? If not, I may be able to in the morning. -Chris -- Chris Suich chris.su...@netapp.com<mailto:chris.su...@netapp.com> NetApp Software Engineer Data Center Platforms – Cloud Solutions Citrix, Cisco & Red Hat On Oct 23, 2013, at 5:56 PM, SuichII, Christopher <chris.su...@netapp.com> wrote: My understanding is that it is still a work in progress to get those test back running. Is this correct, Edison? -- Chris Suich chris.su...@netapp.com NetApp Software Engineer Data Center Platforms – Cloud Solutions Citrix, Cisco & Red Hat On Oct 23, 2013, at 5:48 PM, Darren Shepherd <darren.s.sheph...@gmail.com> wrote: I fixed all the compilation errors in engine/storage/integration-test. I don't know how to run those test though, so I can't validate the changes. Darren On Wed, Oct 23, 2013 at 1:53 PM, SuichII, Christopher <chris.su...@netapp.com> wrote: Yep. I’m running on a clean master. -- Chris Suich chris.su...@netapp.com NetApp Software Engineer Data Center Platforms – Cloud Solutions Citrix, Cisco & Red Hat On Oct 23, 2013, at 4:30 PM, Darren Shepherd <darren.s.sheph...@gmail.com> wrote: Okay let me look at that. Are you 100% sure your looking at a clean version of master? Darren On Oct 23, 2013, at 1:17 PM, "SuichII, Christopher" <chris.su...@netapp.com> wrote: Er, sorry. That was poorly worded on my part. Some classes, like SnapshotTest.java and all the storage providers, did not get updated references to your refactoring. They still reference StrategyPriority.pickStrategy(), etc. Additionally, I changed the pickStrategy() logic from using a comparator to looping over the list once keeping a reference to the best result. This logic was lost in the merge. -- Chris Suich chris.su...@netapp.com NetApp Software Engineer Data Center Platforms – Cloud Solutions Citrix, Cisco & Red Hat On Oct 23, 2013, at 4:13 PM, Darren Shepherd <darren.s.sheph...@gmail.com> wrote: The transaction API was changed in the merge. I could have maybe missed updating a class. Let me check. When you said "It looks like the changes from us didn’t make it through your merge at all," can you point to something specific that got lost? Darren On Wed, Oct 23, 2013 at 1:05 PM, SuichII, Christopher <chris.su...@netapp.com> wrote: And it looks like some of your changes may have not merged correctly. I’m getting compile errors like: The method close() is undefined for the type Transaction This shouldn’t have come from our merge. -- Chris Suich chris.su...@netapp.com NetApp Software Engineer Data Center Platforms – Cloud Solutions Citrix, Cisco & Red Hat On Oct 23, 2013, at 3:52 PM, Darren Shepherd <darren.s.sheph...@gmail.com> wrote: Chris, Edison, You guys just committed 'Support Revert VM Disk from Snapshot.' At the same time I was merging both my txn-refactor and spring-modularization branches. They are really tricky merges and each time I have to rebase it takes awhile to figure out. Anyhow, your change + my changes breaks master. So I quickly rebased rb14823 and committed to master. rb14823 is the patch that makes the Storage Strategies work with my spring work plus clean up some things. Additionally I found out you can't inject List<SnapshotStrategy> to the Snapshot object, so we really have to go with my change to centralize the ownership of the strategies to a single class. Can you please pull master and revalidate that I didn't break anything, if its not too much of a pain. Thanks, Darren
