On 07/12/14 11:31, Anton Khirnov wrote:
1) while vertical alignment does in many cases improve readability, its
    absence is not so horrible as to make the code unreadable

I can drop that from the uncrustify ruleset if that's the problem.

2) the second patch in the series does not in any way require reading
    the parts you are reindenting

My vim setup invokes uncrustify as reformatting command so I routinely use it, for this file the result from inspection isn't bad so I though of not having to do it again when I'll have to touch this file for other changes soon I'm afraid (see below).

As we already discussed several times before, overzealous cosmetics is BAD.
It adds more unnecessary cruft to the history, is hard to review
properly and carries a risk of introducing hard to find bugs, since few
(or no) people actually check that the generated code is the same before
and after.

Crufty code is worse and, since git provides already tools to track changes across cosmetic patches, it isn't really a problem to me.

I can document their usage if that concerns you.

What's more, it's not a convergent procedure, since different people
have different views on what is proper indentation for vertical
alignment.

Thus why I keep refining and keep public the uncrustify ruleset to make most of it automated. (and I'm looking forward to have an even better formatter based on cparser or clang)

Of course fixing horribly formatted unreadable files from 2003 is good
and just, but that is certainly not the case here. So please, no
unnecessary cosmetics unless you actually touch the code in question.

Dropping this patch forced me to redo the second.

Soon I'll start sending some chunks of security fixes (since Google kindly provided me another batch of samples exercising faulty code) and I will need to clean the code I'm reading, I hope you'd not force me to redo 200+ patches or have me drop patches cleaning code that I have to read to track where the actual problem is.

lu
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to