> On Oct. 3, 2013, 10:10 a.m., Saksham Srivastava wrote: > > utils/src/com/cloud/utils/net/NetUtils.java, line 366 > > <https://reviews.apache.org/r/14451/diff/1/?file=360597#file360597line366> > > > > Any particular reason for making dot as private static ? Making it > > local to ip2Long could also solve the purpose as I don't see it being used > > anywhere else. > > If u still want to make it as private static it is better to keep the > > variable with all other static variables at the beginning of the class. > >
static: the point is that it should be created only once, avoiding creating the pattern again and again when it is just a local var Pattern is thread safe > On Oct. 3, 2013, 10:10 a.m., Saksham Srivastava wrote: > > utils/src/com/cloud/utils/net/NetUtils.java, line 149 > > <https://reviews.apache.org/r/14451/diff/1/?file=360597#file360597line149> > > > > Can you mention against which branch the patch was created. Also create > > a bug/enhancement in Jira to track the issue. this test is against master branch create a jira ticket for what issue? > On Oct. 3, 2013, 10:10 a.m., Saksham Srivastava wrote: > > utils/test/com/cloud/utils/net/NetUtilsTest.java, line 225 > > <https://reviews.apache.org/r/14451/diff/1/?file=360598#file360598line225> > > > > Can you add few negative test cases like (15, "00:00:00:00:00:ff") in > > each of these methods. Could you add those tests? I have no idea for those tests right now but I will read your patch. - Laszlo ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14451/#review26645 ----------------------------------------------------------- On Oct. 2, 2013, 9:19 p.m., Laszlo Hornyak wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14451/ > ----------------------------------------------------------- > > (Updated Oct. 2, 2013, 9:19 p.m.) > > > Review request for cloudstack. > > > Repository: cloudstack-git > > > Description > ------- > > - tests added for mac2Long, byte2Mac, long2Mac, ip2Long and long2Ip > - isWindows static method removed - it was not used > - minor changes in the implementations of the tested methods > > > Diffs > ----- > > utils/src/com/cloud/utils/net/NetUtils.java 1e72e22 > utils/test/com/cloud/utils/net/NetUtilsTest.java 3cfc98f > > Diff: https://reviews.apache.org/r/14451/diff/ > > > Testing > ------- > > yes > > > Thanks, > > Laszlo Hornyak > >
