Code review comment for lp:~tealeg/landscape-client/sanitise-hostname

Revision history for this message
Geoff Teale (tealeg) wrote :

> +1!
>
> [1] Probably can boil down this regex a bit more:
>
> 14 +HOSTNAME_REGEXP = re.compile(
> 15 + "^(([a-zA-Z]|[a-zA-Z][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)"
> 16 + "*([A-Za-z]|[A-Za-z][A-Za-z0-9\-]*[A-Za-z0-9])$")
>
>
> So I think the regex could be:
>
> "^(([a-zA-Z][a-zA-Z0-9\-]*)?[a-zA-Z0-9][\.]?)*"
> "(([A-Za-z][A-Za-z0-9\-]*)?[A-Za-z0-9])$"

Looks good! Done.

>
> [2] I get merge conflicts in the translation files that you'll probably
> resolve on commit.
> patching file po/fr.po
> Hunk #1 FAILED at 8.
> Hunk #2 FAILED at 56.
> Hunk #3 FAILED at 124.
> Hunk #4 succeeded at 150 (offset 1 line).
> 3 out of 4 hunks FAILED -- saving rejects to file po/fr.po.rej
> patching file po/landscape-client.pot
> Hunk #1 FAILED at 8.
> Hunk #2 FAILED at 55.
> 2 out of 2 hunks FAILED -- saving rejects to file po/landscape-client.pot.rej
> patching file setup.cfg
>
>
>
> Too bad underscore is part of \w, would have made things look a lot simpler.

OK. I'll make sure this is right when I merge.

Thanks!

« Back to merge proposal