[Development] Qt Coding Guidelines

Rutledge Shawn Shawn.Rutledge at theqtcompany.com
Fri Mar 18 09:24:41 CET 2016


On 18 Mar 2016, at 08:48, Jędrzej Nowacki <jedrzej.nowacki at theqtcompany.com<mailto:jedrzej.nowacki at theqtcompany.com>> wrote:

Every time someone discuss coding style issues my blood boils. I understand
that it is important to have consistent coding style, but discussing where to
put braces or spaces is just waste of developing time.

Agreed, but that’s what code review is all about, as it turns out.  </sarcasm>

I'm totally for an automated solution. I would even say that every rule
formatting related which is not expressed in some tooling aware code should
disappear. In addition, I really do not like that sanity bot is complaining
about typos after I pushed patches to gerrit and even worse it is commenting
on my code instead of proposing a fix.

Well, it would be nice if gerrit allowed reviewers to propose fixes too, so that you can just accept or reject individual proposed diff lines without having to do a git checkout to get exactly back the same state (which means you first need to commit or stash whatever you were doing, because it will be interrupted by fixing nitpicks), fixing the nits and pushing again.  Just like you can edit the commit message right on gerrit.  But, who’s going to implement it...

As you said coding convention are a bit bigger topic, but it also should be
automated. Seriously, rules about where to place Q_DECLARE_METATYPE or a check
if an include is missing are quite easy to express.

So I think, that we should not discuss what is better qdoc or md. The real
discussion is about tooling, what is the best tool to sanitize Qt code. We
need something that:
1. Can work as a sanity bot
2. Can re-format the code by applying changes (git hook?)

Forcing it on everyone that way will be controversial, because there is still some leeway in formatting, whereas automation would remove any chance of personal preference, and probably screw up in some cases.  But we could at least start by adding the clang-format config file to git so that it’s available for doing this manually.  Then in a few years maybe everybody will get used to it enough, and it can be refined enough, to become near-universal.

Otherwise there is the problem of running it for the first time, which the nitpicking reviewers would insist should be a separate commit.  So somebody would probably want to run clang-format on the whole repo, and make one big formatting-fix commit, which brings up said opportunity for clang-format to screw up alignment of code and comments in ways that will piss people off.  So I guess it would have to rather be done one file at a time, but due to the rule that we can’t fix whitespace and code at the same time, that would result in lots of extra noise in the commit logs: clang-format this.cpp, clang-format that.h and so on.

Not fixing code and whitespace in the same patch is a stupid rule of ours IMO.  (It’s OK to fix the copyright header at the same time, but never dare to remove or add a single misplaced space at the same time.)  I think we should abandon that rule and allow formatting fixes at the same time as code fixes, especially if the formatting fixes are being done by some tool anyway.  Especially if gerrit could have a checkbox to ignore whitespace changes, like most offline diff viewers do.  Then when reviewing you look twice: once to see the code changes, and once to ensure that the whitespace changes are sane.  (But the sanity bot already does that.)

3. Rules are easy to express and they can be exported (qdoc, html, fooBar)
4. Works on diff level (so it doesn't complain about the whole world being
broken)

Bonus:
5. C++, js, qml awareness

Cheers,
Jędrek
_______________________________________________
Development mailing list
Development at qt-project.org<mailto:Development at qt-project.org>
http://lists.qt-project.org/mailman/listinfo/development

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20160318/7a102ae1/attachment.html>


More information about the Development mailing list