test_source harmful?

[Robert Collins]

On Fri, 2009-02-27 at 14:08 +1100, Ben Finney wrote:
> Robert Collins <robert.collins at canonical.com> writes:

> > Sound like you agree: we should not have the whitespace check in
> > test_source, instead we should deal with any small issues that
> > happen - and file bugs to provide feedback on where bzr needs to
> > improve.
> 
> This assumes that Bazaar is the only tool that will be comparing these
> files, but that's not true. Dealing with trailing white space in one
> tool doesn't help other tools.

Possibly. If trailing whitespace was hurting us though, surely we'd have
had lots of complaints, instead of occasional grumbles.

> If trailing white space is a problem (which is for a given team to
> decide), it makes the most sense for them to eradicate it from the
> code base and have an enforced policy to prevent it getting back in.

I disagree here. If its a problem you can look at how and why its a
problem and decide what approach to take to solving it. gnu diff
provides tools to strip it from diffs - thats the appraoch of making the
toolchain more powerful, rather than enforcing a strict policy on the
developers. IME strict policies on the developers need strong reasons
behind them for the developers to buy into them.

> > This ignores the issue: Trailing whitespace is not structurally bad;
> > it doesn't cause bugs; it might _rarely_ cause merge conflicts.
> > Checking that something unimportant is set to a particular value is
> > still checking something unimportant.
> 
> Is it true, though, that the Bazaar team considers trailing
> whitespace, and the problems caused by it, to be unimportant?

I've not asserted that its unimportant; I've asserted that its not
something the 'bazaar test suite' should test. *If* it causes problems I
think we should consider that a bug in bzr's merge|annotate|diff|
whatever support and fix that.

> > I don't want to be trained to jump though an unnecessary hoop: I
> > want to remove the hoop.
> 
> Again, that's obviated (while preserving the enforced policy) by
> configuring your text editor to remove trailing white space
> automatically.

My editor does that very well, but with subtly different rules to that
introduced by the source code formatting tests. If I felt that the
policy deserved to be as strict as the test makes it, I would spend the
time figuring out how to make those rules the same for bzr while
different for other projects. But that doesn't seem like a good use of
my time.

> > But trailing whitespace doesn't matter in terms of quality any more
> > than too much or too little VWS matters. We don't have tests for
> > those things because they don't matter as far as the test suite is
> > concerned. The test suite exists to serve us in developing robust
> > high quality code for our users.
> 
> That's the question. Does the Bazaar team consider trailing white
> space to be a quality regression?

I don't see how it [trailing whitespace in the sourcecode of bzr itself]
can be a quality regression as it has no functional impact on the
behaviour of bzr.

-Rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20090227/ff5549af/attachment.pgp