Good suggestions. No rush, so I'll pick this up tomorrow
On 12 May 2014 18:23, "Alex Heneveld" <notifications@github.com> wrote:

> In core/src/main/java/brooklyn/location/basic/BasicOsDetails.java:
>
> > @@ -51,8 +51,8 @@ public boolean isWindows() {
> >
> >      @Override
> >      public boolean isLinux() {
> > -        String unices = "centos|debian|fedora|gentoo|linux|rhel|slackware|solaris|suse|ubuntu";
> > -        return getName() != null && Pattern.matches(unices, getName().toLowerCase());
> > +        String unices = "(?i)centos|debian|fedora|gentoo|.*linux.*|rhel|slackware|solaris|suse|ubuntu";
>
> a few comments. would be nice to:
>
>    - make unices a public static final on the class eg
>    UNIX_OS_NAME_PATTERNS
>    - put .*linux.* first
>    - add coreos ?
>    - include comment that (?i) makes it case insensitive (i presume
>    that's what it does ... advanced regex-fu!)
>
> happy to merge w/o this if needed but would be nice to apply the above
> while it's fresh so leaving this open for a bit
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/brooklyncentral/brooklyn/pull/1380/files#r12541801>
> .
>