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

Revision history for this message
Raphaƫl Badin (rvb) 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.

And you'll admit that the conversion to a string is a one-liner and does not deserve a method in itself.

But ok, I guess we can revisit this code if we need the flexibility I'm talking about later on.

« Back to merge proposal