On Thu, Dec 19, 2013 at 5:35 PM, Jeff Gilbert <jgilb...@mozilla.com> 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) > There's a lot of tight coupling scattered through the engine. For example, consider all the places that interact with the memory reporting system, or the cycle collector, or nsIXPConnect. When njn or mccr8 or I need to modify something related to one of these things, it very often involves cross-cutting changes. And generally ones in which we and our peers are actually the experts, not the owners of the modules we touch. I know some people break up patches into tiny pieces to get rubber-stamp from each module owner. But I don't believe that's a great use of time if the change doesn't actually concern something with which the module owner has particular expertise. > 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. > We're never going to reach consensus on what's pretty. I think we should bring up any issues for discussion, hear people out, and then delegate the decision to someone like bsmedberg. 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. > As noted, I don't think that treating these decisions as the prerogative of the module owner is beneficial to the project as a whole. > 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 don't think it's realistic to convert all of Gecko to one style (among other things, it would break blame). I think preserving existing style is the right approach. But my point is that we should establish and document an _ideal_ style for all of Gecko, and that module owners should make every effort to converge on that style (even if they don't like parts of it), rather than using the existing discord as an excuse to impose their aesthetic whims on the code under their control. > 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. As someone who works and reviews regularly across 3 different style fiefdoms, thinking about style consumes an unfortunate amount of my 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. > I think we should encouraging and facilitating cross-module hacking wherever we can. bholley _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
