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)
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. 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. If we're going to standardize a certain style for the whole tree, then we're going to need to re-discuss a couple of the rules. It's a decent amount of work to restyle the modules well, and I'm hardly eager to do work to make this code *less* readable for the only people who ever look at it. 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. Besides, if style is important enough to standardize, surely it must be important enough to fully address. 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. 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. 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. -Jeff 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. ----- Original Message ----- From: "Bobby Holley" <bobbyhol...@gmail.com> To: "Jeff Gilbert" <jgilb...@mozilla.com> Cc: "Ehsan Akhgari" <ehsan.akhg...@gmail.com>, "Till Schneidereit" <t...@tillschneidereit.net>, "Martin Thomson" <m...@mozilla.com>, "Gregory Szorc" <g...@mozilla.com>, "Nicholas Nethercote" <n.netherc...@gmail.com>, "Andrea Marchesini" <amarches...@mozilla.com>, "Mike Hommey" <m...@glandium.org>, dev-platform@lists.mozilla.org Sent: Thursday, December 19, 2013 2:56:28 PM Subject: Re: On the usefulness of style guides (Was: style guide proposal) On Thu, Dec 19, 2013 at 2:29 PM, Jeff Gilbert <jgilb...@mozilla.com> wrote: > I posit that there's probably not much benefit to having, say, style for > dom/ match style for gfx/, since precious-few people often deal with both > at once. > I don't agree at all. I own XPConnect, which is one of the most stylistically-difficult modules, because it sits at the intersection of the JS engine and the rest of Gecko, which have two very-different style guides. XPConnect generally uses JS style, but there are various Geckoisms that creep in as well. Trying to maintain consistency is a nightmare, especially because there are often cross-cutting patches from JS hackers and Gecko hackers that don't always get review from an XPConnect peer. I myself very often touch code all over the engine in a single patch, and it's impractical to get reviews from all the different owners. Differences between SpiderMonkey style and Gecko style already make this difficult. So I don't think we should try to proliferate this pattern. The JS team has done a great job of documenting very clear style rules in their style guide. I'm pushing to do more of that for Gecko, so that I have clear rules to follow and enforce regardless of whether I'm in content/xbl, dom/base, or image/. Spending time quibbling about these things for every patch is a huge waste of everyone's time. If the rest of a given file is clearly written in a different style, we should try to maintain consistency. Absent that though, we should strive to have one uniform style guide for Gecko, and make sure that it answers all of our questions. bholley _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
