[Development] White space / coding style patches welcome?

[Oswald Buddenhagen]

On Tue, Mar 12, 2013 at 03:14:43PM +0100, Axel Waggershauser wrote:
> On Tue, Mar 12, 2013 at 2:11 PM, Oswald Buddenhagen <oswald.buddenhagen at digia.com> wrote:
> > if the burden on the reviewer is too big, then you didn't split it up
> > according to directories/modules sufficiently well.
> 
> What would be a sensible patch-size? Is there some reasonable maximum
> number of lines/hunks?
> 
try to go by subdirectories of src/ and tools/ first. if the result
looks utterly unreasonable (more than maybe two thousand lines according
to git diffstat), consider further splitting up. and vice-versa if you
get some very small diffs.

> > i did this exercise a few years ago already (just to have it discarded),
> > and the outcome wasn't *that* huge (though there were some problem
> > areas, like qtmultimedia and qt3support).
> 
> You mean, you prepared clean up patches and they were abandoned or
> their effect vanished over time due to missing online checks?
> 
it went to the trash because the idea was not liked back then. the winds
appear to have changed meanwhile.

> 1) 'simple' line-by-line checks running directly on the diff:
> 
qt/qtrepotools/git-hooks/sanitize-commit

> 2) proper c++ parser based checks that require the patch to be applied
> to the code to even run:
> 
this would equate a reliable automatic source reformatter. i've yet to
see one. ^^

> I don't know about the gerrit sanity checking infrastructure but can
> imagine that would require a substantial change
>
it would be entirely different from the sanity bot, yes. but we already
have the doc bot that does something similar.

> and it would also not work without having a fixed source base first.
>
the full results could be filtered with the diff lines, so even an
unclean baseline would produce very few false positives.

> But obviously be able to do proper checks of any valid c++ code
>
to reliably parse c++, you need contextual information. that means that
you need a complete build system. iow, you need to compile the whole
thing, and have the style checking be done as a side effect. 

> (I hereby presume there is some fancy tool in the context of libclang
> available...).
> 
if not: have fun. ^^