On Tue, Mar 12, 2013 at 2:11 PM, Oswald Buddenhagen <oswald.buddenha...@digia.com> wrote: > On Tue, Mar 12, 2013 at 01:34:54PM +0100, Axel Waggershauser wrote: >> I thought splitting by type first and potentially by directory second >> if it is too big still, would be best because it reduces the burden on >> the reviewer: only one type of change should be easier to skim over >> > the downside of doing it by type is producing a lot of history noise - > commits you need to skip over when you are researching history.
Point taken. > 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? > 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? > ok, let's modify the requirement: you don't need to set out to fix every > problem. but in the lines you touch because of fixing specific problems, > i want to see the remaining issues fixed as well. > specifically, i want trailing whitespace and tabs/indentation fixed. > the issue you started with is kinda optional for me. Ack. > spaces around binary operators are kinda secondary, too. i don't even > know how big the problem is. Well, it was totally random but I liked that it was kind of a minor issue resulting in a still smallish patch, perfect for a test balloon. And that patch actually fixes all switch statements in qtbase. Two thoughts/directions regarding support for automatic (sanity-check like) tests: 1) 'simple' line-by-line checks running directly on the diff: That should be rather easy to add as a more or less ad-hock string-procssing check that looks for specific coding style problems on a line-by-line basis in the diff, e.g. detecting classics like "for( ... )" or even more obvious tabs and trailing whitespace issues. Something like that could be integrated anytime without having a complete conformant code base. But it will easily reach it's limits like checking a multi-line-statement and not having any clue whatsoever about the context (complaints about random text in comments, maybe macro foo, etc.) It will obviously be able to only address a subset of what is talked about in http://qt-project.org/wiki/Qt_Coding_Style but maybe a substantial portion and that without too much effort. 2) proper c++ parser based checks that require the patch to be applied to the code to even run: I don't know about the gerrit sanity checking infrastructure but can imagine that would require a substantial change and it would also not work without having a fixed source base first. But obviously be able to do proper checks of any valid c++ code (I hereby presume there is some fancy tool in the context of libclang available...). I'd be inclined to look into option 1 rather than 2 as this seems to be more manageable within a decent amount of time. - Axel _______________________________________________ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
