On 12/19/2013, 8:35 PM, Jeff Gilbert wrote:
How do patches get fully-reviewed if the sum-knowledge of the reviewers doesn't
include whether the style's right? Alternatively, who is signing off on the
content of code, having in-depth knowledge of how it works, but not knowing the
style? That sounds like a different problem, really. (I've also definitely seen
this, fwiw. A number of times, I've seen mis-styled code that should have
failed review for other reasons. Not that I'm saying this is a usecase of
differing styles! Rather, mis-styling correlates with poor review quality)
The patches get reviewed by people who have varying degrees of attention
to coding style. I think that explains what we've ended up very well.
It doesn't necessarily have anything to do with poor reviews.
That aside, what you describe sounds like 'too many' styles, almost definitionally, if
it's such a major pain. However, there are quantities between 'one' and 'many', so I
don't think that because 'too many' is clearly bad, we have to choose 'one'. It's easy to
say "we should all use the same style" before we agree what that ideal style is.
I think this misses the point behind having style guidelines in the
first place. The *point* of having style guidelines is to 1) document
good (and not necessary the absolute best) coding practices and 2) to
harmonize everybody who writes code based on that. Obviously the more
parts of the tree you look at, the more you're going to appreciate the
effect of everything following the style guidelines.
But to address the main point of this paragraph, what's wrong with
having *one* style that *everybody* follows? I can't tell if you have
something against that, or if you just care about a small subset of the
tree and are happy with the status quo in that subset.
Personally, there are a couple of things I don't like about moz-style (though
revisions to the central style guide at least have made it better than it used
to be), but instead of bikeshedding the central style guide, we just do our own
thing in the code we're responsible for.
That is not very helpful. If there is something in the mozilla style
guide that you think is wrong and needs to change, *please* bring it up.
If you're right, you'll be benefiting everyone. And if it's just a
matter of taste, perhaps you could sacrifice your preferences in the
interest of the greater good?
If we're going to standardize a certain style for the whole tree,
Just so that we're clear, we _already_ have a standard style for the
whole tree. It's just that people don't follow it.
> then we're going to need to re-discuss a couple of the rules.
Again, *please* let's have that discussion. You keep mentioning these
two rules but nowhere in your email you're directly saying what they
are. (Sorry if it's something obvious that I didn't figure out!)
> It's a decent amount of work to restyle the modules well
That's actually not true. There are tools which are very good at doing
this work once we agree that it should be done.
>
and I'm hardly eager to do work to make this code *less* readable for
the only people who ever look at it.
Why do you think that there are a small subset of people who ever read
that code?
>
I'm personally fine with bikeshedding over this, but I'm not going to
jump on the 'standardize' bandwagon if we're just going to be told not
to bikeshed later when counter-proposals are brought up. There needs to
be compromise one some front.
I don't think that anybody is suggesting that we come up with a set of
style guides and carve them into stone and never consider anything
otherwise. But then again debating where the * in a pointer notation
ends up with every week isn't the best use of everybody's time. If and
when someone finds something wrong in the style guideline they can bring
it up and get the style modified if they have a good point. Note that
this is quite doable, as evidence of other projects which do this well
shows.
Besides, if style is important enough to standardize, surely it must be
important enough to fully address.
It definitely is!
The last bit of the problem is that *all of Gecko* is currently 'files with
existing styles', so I'm not sure how that can mesh with having 'one true
style' unless we have an initiative to actually convert over all mis-styled
files.
To be clear, I'm *very* interested in updating all of our code to bring
it up to a set of Mozilla styles.
If we either fail to convert large parts of Gecko, or fail to address concerns
people have, we're just going to end up with the same style fiefdoms we already
have.
We can and should definitely avoid both of them.
I suppose my counter-question is 'How does standardizing styles across modules
help us?' In my experience, reviewing (or being reviewed) for style takes
almost no time. It's definitely dwarfed by the time I need to spend thinking
about the changes being made to the code. I think readability improvements (for
those who actually work on the code in question) are more important than the
code looking exactly the same as code from a module I've never even opened.
Here's an attempt at an answer: It makes the project as an entirety be
easier to read, understand, modify, and maintain by more people for an
extended period of time.
And yes, this _does_ mean that people who only look at a subset of the
code won't get much benefit out of it. But as long as everybody agrees
that nobody's personal preferences should necessarily win over others'
depending on who last modified a given file, then that should not be a
problem. Again, there are lots of projects which successfully enforce
consistent style across their code base and people still manage to hack
on those codebases and enjoy doing that!
PS: Don't get me wrong: Moz-style has gotten way better recently. I agree with
almost all of it, but there are a couple of points with which I really don't.
Again, let's discuss those points please!
Cheers,
Ehsan
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
