Sheng, Some of this code will be used in a feature allowing vpc gateways to be plugged to sdn networks and to allow multiple gateways to share that network. I am using that in a hack on 4.1.1 and porting the code to master now, bit by bit so to speak. This is why you see some functions that seem not to be used.
regards, Daan On Wed, Aug 7, 2013 at 8:52 AM, Daan Hoogland <daan.hoogl...@gmail.com>wrote: > Hi Dave, I see what you mean by colons wouldn't be allowed. Nothing > changed but I went for this solution for consistency (over backward > compatibility). I will change again. Let me get to work first. nice working > around the globe;) > > Daan > > > On Wed, Aug 7, 2013 at 3:57 AM, Sheng Yang <sh...@yasker.org> wrote: > >> This is an automatically generated e-mail. To reply, visit: >> https://reviews.apache.org/r/12849/ >> >> On August 6th, 2013, 11:40 p.m. UTC, *Sheng Yang* wrote: >> >> >> api/src/com/cloud/network/Networks.java<https://reviews.apache.org/r/12849/diff/5/?file=337785#file337785line135> >> (Diff >> revision 5) >> >> public String scheme() { >> >> 95 >> >> // do we need to check that value does not contain a scheme >> >> 135 >> >> return new URI(scheme,value.toString(),null,null); >> >> 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. >> >> On August 7th, 2013, 1:45 a.m. UTC, *Sheng Yang* wrote: >> >> Dave explained this. Please drop this issue. >> >> On August 7th, 2013, 1:54 a.m. UTC, *Dave Cahill* wrote: >> >> Thanks for the quick reply Sheng! >> >> Actually, I dug deeper and realized that if we use the 4-parameter URI >> constructor, the "value" parameter is interpreted as host, and colons >> wouldn't be allowed! >> >> It looks like the only way to preserve the behavior as it was is to stick >> with the old string concatenation behavior of scheme + "://" + value. In >> fact, this is what Daan and I agreed on in IRC yesterday, but it looks like >> he went with the 4 param constructor - I'll discuss with him later to see >> what changed. >> >> Thank you Dave! >> >> >> - Sheng >> >> On August 6th, 2013, 2:25 p.m. UTC, daan Hoogland wrote: >> Review request for cloudstack, Chiradeep Vittal, Dave Cahill, Koushik >> Das, and Sheng Yang. >> By daan Hoogland. >> >> *Updated Aug. 6, 2013, 2:25 p.m.* >> *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) >> >> View Diff <https://reviews.apache.org/r/12849/diff/> >> > >
