[haiku-development] Re: Set*UIColor Version 3b Patch
- From: "Adrien Destugues" <pulkomandy@xxxxxxxxxxxxx>
- To: haiku-development@xxxxxxxxxxxxx
- Date: Thu, 19 Nov 2015 16:02:14 +0000
The vast majority of these changes are things I find all over the place in
the Haiku source code.
Usually in the same file upon which I'm working. I just copy the style I see.
And, sometimes,
there's an errant keystroke or a minor misspelling in a commit entry. These
are, undeniably,
pedantic issues. So I spend the time addressing these tiny little issues,
since Axel obviously took
the time to point them out. These are issues that don't affect anything of
consequence, and that
includes code cleanliness or legibility. I've never seen a project be so
picky about such minor
issues as these. Probably because they are all trying to actually get the job
done.
Another "big" project I work with is WebKit. They reject patches (automatically
with a script doing the checks, which we should do as well), for example if you
forgot the final dot at the end of a sentance in a comment.
I mean, seriously, we're talking about 40 or so minor style issues in 10,000
lines of code in a
17,000 line patch file.
I have no problems making these types of changes at all, even if I have to
rewrite half the patch
(which I've done twice - partly in search of a bug I found in existing code).
The more I make of
them, the more natural it will be for me to simply use the expected style
from the beginning, so
all is certainly not lost. I've made it quite clear that I will do what it
takes, but worrying
about all of the tiny inconsequential style issues first and foremost has
stalled meaningful
conversation about how the patch actually works and performs.
And that's the rub, prioritization doesn't exist here. The priority should be
discussing BColorMap
and any publicly visible changes as well as any stability or compatibility
issues, not minor
styling issues.
I'm sure Axel is so trained to the guidelines that he can't read and understand
any code that doesn't follow them. So, this is what he sees first, before he
can dig into the functionality.
We should improve our checkstyle scripts to detect more of the issues, so this
can be checked easily while you write the code, instead of when it is reviewed.
It would save time for everyone.
Maybe we can set up GCI tasks about improving the scripts or working from some
other tools? (I know webkit has a more advanced script but it doesn't match our
coding guidelines, of course).
The script could also be added to our code review system (currently Trac, but I
still hope we will some day setup Gerrit or a similar tool) and automatically
review patches submitted there.
--
Adrien.
Other related posts:
