Code review comment for lp:~allenap/maas/fix-makefile-write-dhcp-stuff

On Friday 08 Jun 2012 12:49:33 you wrote:
> Review: Approve
>
> Looks good. +15/-66 without loosing functionality, that's nice.

'losing'

> Maybe we can wait for Julian to clarify things about [0] but I'll leave it
> up to you because, in my opinion, this is all good.
>
> [0]
>
> > Removes the write_dhcp_config script entirely. It's not necessary -
> >
> > use bin/py -m provisioningserver/dhcp/writer instead - and is
> > misleading because packages cannot use it anyway as it is generated
> > by buildout.
>
> I agree with you. Since Julian is away I can only speculate here but this
> script can only be a development convenience and if it's the case then
> using the command as you suggest is, I think, good enough.

I'm kinda unhappy about this change, I'd have at least expected you to talk to
me before just removing my work. I put the generated script in as a
development convenience, because the bin/py example above is pretty hideous to
remember and type. I intended the package to use the module's script as-is
however, because it will have set up paths already.

Also, the branch has removed a test which checks the stdout output, and the
new name "writer" is inaccurate. If you want to split hairs, it's
"write_config".

I'd appreciate it if you'd put the script back, either buildout-generated or a
shortcut using the bin/py method.

Why did you remove the build dependency from the check target? Regardless of
my changes it seems a good idea.