[Openchrome-devel] [Bug 93243] [Patch] VIARestore function improvement

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Tue Dec 15 16:56:29 PST 2015


--- Comment #5 from Mario Rugiero <mrugiero at gmail.com> ---
I'm with you about the comments, both in style (C89 comments, since just a
little comfort in the comments is not a good reason to drop standard C89
support) and content (we are lacking a lot of info in the driver).
On the return type style, while I consider it ugly, there are a few things
about it that tend to justify keeping it.
First, several system-level projects use that style (sections of the Linux
kernel do, for example, as most of X.org AFAIR), so the argument that it is not
common isn't really that strong.
Second, consistence matter. A good style is twice as good if it is consistent,
but a bad style is twice as bad if it is not. This may be a bad style, but
unless we first do a separate (so we don't mask actual functional changes)
commit fixing it, we'll make it worse if we use the "right" style here and
there, and the "wrong" style everywhere else. Your mind will automate how to
interpret the structure of text if it is consistent. This automation will be
broken if it isn't.
Third, it allows for longer, more descriptive return types without hurting
readability too much. Again, your eyes get used to the fact function signatures
are at least two lines long, but it won't get used to the fact that randomly
sometimes you need to scroll right and sometimes you don't.
Fourth, the more you highlight the important concepts, the more clear the
meaning of the code is, and certainly the return type of a function is
important, and certainly having its own line highlights it.
Aesthetically, I hate that style. But it does have some advantages that should
be weighted out.
Lastly, I suggest we keep conceptually different patches separate from each
other. You can make them depend on each other, but a single huge patch is
always a future problem. We should make stylistic changes in one patchset,
organized by style issue cleaned or by file touched (not both at the same
time), bugfixes one per patch, features on their own patchset, one per patch.
That's what I believe to be common sense.

You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/openchrome-devel/attachments/20151216/7d12b4fd/attachment-0001.html>

More information about the Openchrome-devel mailing list