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. lu _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
