Your remarks on the assert resembles a comment in the code that I didn't write ;)
But seriously; If the string contains a scheme that is not in the enum, you want a exception to be thrown instead of UnDecided or UnDefined? regards, On Sun, Jul 21, 2013 at 5:32 PM, Alex Huang <alex.hu...@citrix.com> wrote: > 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> 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] > *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> 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/>**** > > **** > > ** ** >
