----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12849/#review24756 -----------------------------------------------------------
api/src/com/cloud/network/Networks.java <https://reviews.apache.org/r/12849/#comment48880> I don't quite understand this part. Why "//" is missing in this case? Who would require "//" and who wouldn't? api/src/com/cloud/network/Networks.java <https://reviews.apache.org/r/12849/#comment48877> I think by following the previous case, it should be URI(scheme,value.toString()) rather than "null, null". It possibly doesn't make difference, but it's better not to use different constructor. The same with several URI constructor above. And I doubt if it's really need to discard this check. At least they would working for other broadcast domain type. api/src/com/cloud/network/Networks.java <https://reviews.apache.org/r/12849/#comment48879> No reference of getTypeOf(URI) in the code? api/src/com/cloud/network/Networks.java <https://reviews.apache.org/r/12849/#comment48878> Does this function need to be public? I think getSchemeValue() and getTypeOf(String str) should be enough? - Sheng Yang On Aug. 6, 2013, 2:25 p.m., daan Hoogland wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12849/ > ----------------------------------------------------------- > > (Updated Aug. 6, 2013, 2:25 p.m.) > > > Review request for cloudstack, Chiradeep Vittal, Dave Cahill, Koushik Das, > and Sheng Yang. > > > Repository: cloudstack-git > > > Description > ------- > > Both BroadcastDomainType and IsolationType needed some extra code for > backwards compatibility > > > Diffs > ----- > > api/src/com/cloud/network/Networks.java c76c3d4 > api/test/com/cloud/network/NetworksTest.java 31114e8 > > Diff: https://reviews.apache.org/r/12849/diff/ > > > Testing > ------- > > > Thanks, > > daan Hoogland > >
