launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

[...]
> Several things need attention in the details of the text, in my view:
> 
>  * It doesn't help that the "this is perfectly fine" is ambiguous -- it could
> refer to the inability to parse the leases file, or to installation of maas-
> dhcp.
> 
>  * Is "Unable to parse the DHCP leases file" really the best information we
> can give?  There's a big difference between "the leases file does not exist"
> and "the parser had a problem with the contents of the leases file."  I think
> this message is only meant for the former case.
> 
>  * Hinting that people may be able to solve their problems by installing maas-
> dhcp is an open invitation for people to set up accidental rogue DHCP servers.
> 
>  * "Note that" doesn't carry a lot of meaning.  Better leave it out.
> 
> Please fix up before landing!
> 
> Jeroen

You're definitely right… how about:

        task_logger.info(
            "The DHCP leases file does not exit.  This is only a problem if "
            "this cluster controller is managing its DHCP server.  If that's "
            "the case then you need to install the 'maas-dhcp' package on "
            "this cluster controller.")



-- 
https://code.launchpad.net/~rvb/maas/bug-1061409/+merge/127979
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/bug-1061409 into lp:maas.