Code review comment for lp://staging/~julian-edwards/gwacl/xml-definitions

Revision history for this message
Gavin Panella (allenap) wrote :

On 12 March 2013 08:23, Raphaël Badin <email address hidden> wrote:
>> I genuinely don't understand what your suggestion buys us. Feel free to
>> convince me more, but I want to see concrete advantages. As it stands,
>> the way it's done encapsulates some formatting (IME this is *extremely*
>> important for debugging at a later date)
>
> My point was that, by encapsulating the creation of the xml in one
> method like that, we force all the callsites to use the same
> formatting. If we were not doing that we could: use the indented
> version in the tests for maximum readability, use the non-indented
> version in production where 'debug' (a config setting somewhere) is
> off, use the indented version in production where 'debug' is
> one. What I'm saying is that the code right now slightly limits our
> ability to adapt the formatting to different situations.

I think it's better to stick to one representation, and the pretty
version would be my choice. There's no need for the condensed version
because we can always use compression over the wire. The complexity of
adding a config option, while small, is not compensated by the limited
usefulness of being able to choose between pretty and ugly printing.
The former will also come in handy when debugging a live system.

« Back to merge proposal