Daan, I'm not sure I understand what you're saying here.
--Alex From: Daan Hoogland [mailto:daan.hoogl...@gmail.com] Sent: Sunday, July 21, 2013 4:23 AM To: Alex Huang Cc: Hugo Trippaers; cloudstack Subject: Re: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs Alex, I saw you put you comments in the commit as comment...??? why not as code then? On Sat, Jul 20, 2013 at 3:42 PM, Alex Huang <alex.hu...@citrix.com<mailto:alex.hu...@citrix.com>> wrote: Daan, It should assert and throw a CloudRuntimeException. https://cwiki.apache.org/confluence/display/CLOUDSTACK/Exceptions+and+logging --Alex From: Daan Hoogland [mailto:daan.hoogl...@gmail.com<mailto:daan.hoogl...@gmail.com>] Sent: Saturday, July 20, 2013 3:25 AM To: Alex Huang Cc: Hugo Trippaers; cloudstack Subject: Re: Review Request 12685: CLOUDSTACK-1532 added utility functions to scan URIs Alex, it falls through to 'UnDecided' if not known. Do you mean it should throw something? or maybe add an extra 'UnDefined'? On Fri, Jul 19, 2013 at 5:31 PM, Alex Huang <alex.hu...@citrix.com<mailto:alex.hu...@citrix.com>> wrote: This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12685/ Ship it! master: 2d4464d One review comment is if you are checking for the colon to see if the value has the schema already, then shouldn't it also check if the schema matches what's declared? But that shouldn't block this review. The code itself is technically sound. - Alex Huang On July 17th, 2013, 3:46 p.m. UTC, daan Hoogland wrote: Review request for cloudstack and Hugo Trippaers. By daan Hoogland. Updated July 17, 2013, 3:46 p.m. Bugs: CLOUDSTACK-1532 Repository: cloudstack-git Description the review for the complete patch was open to long and no longer applicable, so I am starting with smaller patches as this is really needed to implement CLOUDSTACK-1532 Testing unit testing Diffs * api/src/com/cloud/network/Networks.java (5aede05) * api/test/com/cloud/network/NetworksTest.java (PRE-CREATION) View Diff<https://reviews.apache.org/r/12685/diff/>
