Code review comment for lp://staging/~julian-edwards/maas/forwarders-config-writer

On Tuesday 18 Feb 2014 09:20:13 you wrote:
> > > Question: are we not writing docstrings anymore? That's fine, whatever.
> > > But if we're doing a new thing, nobody told me.
> >
> > Write them judiciously. In other words, does it add value? If it does do
> > it, if not, don't.
>
> You did, however, write some top comments that explained what the methods
> were for — that's exactly what should be in a docstring.

Oh sorry, I see what you mean now. Yes, they were left over from refactored
blocks, which now look like they ought to be docstrings. So now they are.

> > > I see no mention of what it means if you already had a ‘forwarders’
> > > setting.> >
> > > And yet we overwrite it. Worth saying?
> >
> > There's already a comment, did you miss it or did I not write enough?
> >
> > # Remove existing forwarders, it's a syntax error to have more
> > # than one in the combined configuration for named.
>
> That says it's an error to have more than one. But it doesn't say where
> that pre-existing one would come from

Not sure how I could know that :)

>, and why it is safe to delete it.
> Might cause future maintainers to miss subtleties.

I've mentioned that the old value is going to be in the backup file made
(exactly why I was careful about making a backup).

cheers.