You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Darren Shepherd <da...@gmail.com> on 2013/10/04 19:24:29 UTC
Re: Review Request 13938: StringUtils csv methods simplified
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13938/#review26676
-----------------------------------------------------------
utils/src/com/cloud/utils/StringUtils.java
<https://reviews.apache.org/r/13938/#comment51962>
This is not actually correct, the previous code would do a trim() and the new code also can introduce a NPE. So check for null and return empty list, also the following snippet deals well with white space
tags.trim().split("\\s*,\\s*")
- Darren Shepherd
On Sept. 3, 2013, 7:22 a.m., Laszlo Hornyak wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13938/
> -----------------------------------------------------------
>
> (Updated Sept. 3, 2013, 7:22 a.m.)
>
>
> Review request for cloudstack and Prachi Damle.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> - longer method bodies replaced with oneliners
> - tests added to ensure compatibility
>
>
> Diffs
> -----
>
> utils/src/com/cloud/utils/StringUtils.java 948c0ac
> utils/test/com/cloud/utils/StringUtilsTest.java ae37c24
>
> Diff: https://reviews.apache.org/r/13938/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Laszlo Hornyak
>
>
Re: Review Request 13938: StringUtils csv methods simplified
Posted by Amogh Vasekar <am...@citrix.com>.
> On Oct. 4, 2013, 5:24 p.m., Darren Shepherd wrote:
> > utils/src/com/cloud/utils/StringUtils.java, line 79
> > <https://reviews.apache.org/r/13938/diff/2/?file=347449#file347449line79>
> >
> > This is not actually correct, the previous code would do a trim() and the new code also can introduce a NPE. So check for null and return empty list, also the following snippet deals well with white space
> >
> > tags.trim().split("\\s*,\\s*")
Reminder -
Hi,
This request has been pending for long. Please take the time to address the comments raised. Thanks!
- Amogh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13938/#review26676
-----------------------------------------------------------
On Sept. 3, 2013, 7:22 a.m., Laszlo Hornyak wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13938/
> -----------------------------------------------------------
>
> (Updated Sept. 3, 2013, 7:22 a.m.)
>
>
> Review request for cloudstack and Prachi Damle.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> - longer method bodies replaced with oneliners
> - tests added to ensure compatibility
>
>
> Diffs
> -----
>
> utils/src/com/cloud/utils/StringUtils.java 948c0ac
> utils/test/com/cloud/utils/StringUtilsTest.java ae37c24
>
> Diff: https://reviews.apache.org/r/13938/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Laszlo Hornyak
>
>