On Sat, 10 May 2014 18:50:13 +0200, Luca Barbato <[email protected]> wrote: > On 21/04/14 12:43, Anton Khirnov wrote: > > > > On Tue, 15 Apr 2014 05:16:26 +0200, Vittorio Giovara > > <[email protected]> wrote: > >> --- > >> libavformat/mov.c | 1108 > >> ++++++++++++++++++++++++++++++----------------------- > >> 1 file changed, 633 insertions(+), 475 deletions(-) > >> > > > > TBH I am not very happy about this patch. > > > > 1) It does a bunch of things that are not pure reformatting (removing > > commented > > out stuff, reordering headers, etc..). This makes the review much harder, > > because now the reviewer has to check what is actual cosmetics and what > > is > > not. > > So please -- stop doing this. Reformatting patches should contain only > > reformatting and nothing else. Split other forms of cosmetics into > > separate > > patches. > > 2) Overly strict adherence to guidelines makes the code actually _harder_ to > > read in some places (e.g. unnecessary line breaks, splitting one-line > > case) > > 3) Finally, the formatting in this file was mostly fine before the patch. I > > can > > understand (and even write myself) huge reformatting patches for horrible > > mpegvideo-level unreadable crap. But obsessively fixing every single > > formatting nit in every single file you touch is IMO counterproductive. > > > > Automatic cleanups can have this side effect, I wouldn't stall the set > because of that. >
The set is not stalled because of this, Vittorio said on IRC he'd drop the patch. And btw this is one of the reasons I don't believe in automatic cosmetics. -- Anton Khirnov _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
