Checking back in - any progress on this review? We're now at the 3 week point from initial submission.
Hugo, is your Ship It (once test run completes) equivalent to a "commit now", or is there an additional review required? I don't see any other review activity, so I'm guessing you're the only one actively looking at this at the moment. On Fri, Mar 29, 2013 at 11:37 PM, Dave Cahill <dcah...@midokura.com> wrote: > Sounds great, thanks Hugo! > > > On Fri, Mar 29, 2013 at 8:47 PM, Hugo Trippaers < > htrippa...@schubergphilis.com> wrote: > >> Hey Dave, >> >> My "ship it" is waiting for a last compile and test run on my dev >> platform. I'll try to do that over the weekend. >> >> Cheers, >> >> Hugo >> >> Sent from my iPhone >> >> On 29 mrt. 2013, at 02:17, "Dave Cahill" <dcah...@midokura.com> wrote: >> >> Hi all, >> >> I think all review comments have been addressed on this, but review >> progress seems to have stalled - anything I should be doing to keep things >> moving? >> >> Thanks, >> Dave. >> >> >> On Wed, Mar 27, 2013 at 3:33 PM, Dave Cahill <dcah...@midokura.com>wrote: >> >>> This is an automatically generated e-mail. To reply, visit: >>> https://reviews.apache.org/r/9898/ >>> Review request for cloudstack, Hugo Trippaers and Chiradeep Vittal. >>> By Dave Cahill. >>> >>> *Updated March 27, 2013, 6:33 a.m.* >>> Changes >>> >>> Fixed Hiroaki's exception-related review comment. >>> >>> Description >>> >>> Feature >>> spec:https://cwiki.apache.org/confluence/display/CLOUDSTACK/Midokura+Networking+Plugin >>> >>> Jira ticket:https://issues.apache.org/jira/browse/CLOUDSTACK-996 >>> >>> Notes: >>> >>> * Documentation will follow as a separate commit >>> >>> * One main difference from existing networking plugins is the lack of a >>> Resource class; we didn't feel it was necessary in this case. As mentioned >>> in Extending CloudStack Networking [1]: >>> "Just like managers, resources are not strictly necessary. In theory a >>> Network Element could implement a client for the API of the new controller >>> and therefore be completely self-contained." >>> >>> * We allow overriding Public traffic via the MidoNetPublicNetworkGuru. We >>> checked this approach with the list [2] and received no comments, so we're >>> going with it for now. >>> >>> [1] https://cwiki.apache.org/CLOUDSTACK/extending-cloudstack-networking.html >>> [2] http://markmail.org/message/k5qse63eyylszm3i >>> >>> Testing >>> >>> Built and deployed, spun up Advanced Isolated network with two VMs, >>> verified internal and external connectivity via MidoNet. >>> >>> *Bugs: *CLOUDSTACK-996 >>> Diffs (updated) >>> >>> - api/src/com/cloud/network/Network.java (c2ab655) >>> - api/src/com/cloud/network/Networks.java (e3d2158) >>> - api/src/com/cloud/network/PhysicalNetwork.java (343a2b1) >>> - api/src/org/apache/cloudstack/network/ExternalNetworkDeviceManager.java >>> (bc22804) >>> - client/pom.xml (7ad2eff) >>> - >>> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java >>> (b622b6d) >>> - >>> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java >>> (c93aeeb) >>> - plugins/network-elements/midokura-midonet/pom.xml (7f2e2d3) >>> - plugins/network-elements/midonet/pom.xml (PRE-CREATION) >>> - >>> plugins/network-elements/midonet/src/com/cloud/network/element/MidoNetElement.java >>> (48833b3) >>> - >>> plugins/network-elements/midonet/src/com/cloud/network/element/SimpleFirewallRule.java >>> (PRE-CREATION) >>> - >>> plugins/network-elements/midonet/src/com/cloud/network/guru/MidoNetGuestNetworkGuru.java >>> (ed0eb3c) >>> - >>> plugins/network-elements/midonet/src/com/cloud/network/guru/MidoNetPublicNetworkGuru.java >>> (PRE-CREATION) >>> - >>> plugins/network-elements/midonet/src/com/cloud/network/resource/MidoNetVifDriver.java >>> (PRE-CREATION) >>> - >>> plugins/network-elements/midonet/test/com/cloud/network/element/MidoNetElementTest.java >>> (PRE-CREATION) >>> - plugins/pom.xml (39d9907) >>> - server/src/com/cloud/configuration/Config.java (9db7dbd) >>> - server/src/com/cloud/network/NetworkManagerImpl.java (b1236cc) >>> - ui/scripts/system.js (c0a5d14) >>> >>> View Diff <https://reviews.apache.org/r/9898/diff/> >>> >> >> >
