test_source harmful?

[Marius Kruger]

2009/2/25 Robert Collins <robert.collins at canonical.com>:
>> > The whitespace checks are really frustrating me;
I suspected that this will happen. sorry.

>> I thought that was the point: to put the pain of fixing whitespace
>> mistakes squarely where they belong, at the point of the person
>> attempting to introduce them.
yes, don't allow new ones to be added.

> I don't think frustrating people was the point; honestly we 'never' (or
> extremely rarely) had merge conflicts due to whitespace fixes being
> done. Creating pain to solve a non-problem is hard for me to understand.
sorry.

> I don't think we can expect other projects to use a source code check
> like we have done by default. If whitespace conflicts are such an issue
> we should make our merge handle whitespace conflicts better - because
> thats what our users will need.

YES PLEASE!
Our policy at work is to not have trailing whitespace because it is
too difficult to
see what really happend or merge when somebody went crazy with
whitespace (lots of accidental changes; most programmers don't check
their diffs before committing)

>> > I don't think we suffered anywhere near as much when occasional
>> > trailing whitespace existed as we do when merges are rejected [after
>> > a long test process]
>>
>> Perhaps the checks on source should be done before running any tests
>> of functionality. Is that sequencing possible?
>
> It is,
pqm could do:
bzr selftest --first test_source

> but it still means a round trip [context switching etc].

thats why I made the bzr-text-checker plugin to warn you about it at
commit time, or prevent you from committing it in the first place.
https://launchpad.net/bzr-text-checker
Content filtering should be able to remove it automatically for you in
the future.

>> > I'd like to delete the test for source code formatting; unlike
>> > asserts trailing whitespace is not itself a bug.
well if we don't catch them at test time, trailing whitespace will get added.
I agree that you don't want to figure this out after a long test run,
but this will  train you to check it beforehand with `bzr cdiff
--check-style` or running the test on your side or something.

>> Isn't the solution simply to configure your text editor to remove
>> trailing whitespace before you save the file?
>
> Thats one approach.
in order to do that, you have to know all the source does not have
trailing whitespace to start off with,
otherwise you get lots of 'spurious whitespace' changes when editing a
file. That is the point of this test:
to guarantee that there is no trailing whitespace in the code, so that
you can easily use your
editor to manage it for you.

sorry for the pain, but I think its worthwhile for bzr to figure out
how to solve this sort of problems,
which would be useful for the users in the end (who might need it for
other projects too).

regards
marius