Quoting Luca Barbato (2014-12-07 13:31:54) > 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. >
The problem here is not that I'm against vertical alignment. The problem is that the overhead of adding it everywhere is simply not worth it. > > 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). So fix your vim setup. Reformatting every file you ever touch is a very bad practice, for reasons stated in the previous mail. > > > 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. This is not crufty code. As I already said, the formatting in this specific file (and these days, in most other files as well) is mostly fine and perfectly readable. > > I can document their usage if that concerns you. > That is completely orthogonal to what I'm talking about. > > 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) > Again, this is completely orthogonal to what I'm talking about. Some things simply are up to people's personal preferences. I very strongly disagree with automatically reformatting code with some tool all the time. > > 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. 1) This just reinforces my point of avoiding unnecessary cosmetics, since it breaks other people's work in progress for no good reason. 2) You should not be hoarding 200+ patches, unless they all depend on each other. For exactly this reason. -- Anton Khirnov _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
