A concise summary of the changes you're proposing would be useful - here's my attempt at one.
>From what I gather, the changes you're proposing to the style guide are: * remove implicit discouragement of changing code to conform to "Mozilla style" ** style changes should never be combined with functional changes ** (other specifics about how this should be accomplished and with what exceptions may need elaborating) * remove the text suggesting that module owners can dictate different code styles I don't have any objection, and think that these policy changes are reasonable. I think the work that these policy changes entail (i.e. individual style fix patches) should still be evaluated on its own merit by module owners/peers, and that may mean that the style fixes are treated as a lower priority than other work. I think we should also do a review of the style guide before we decide to try to enforce it more consistently. I've done that quickly and have noticed a couple of issues - I will post separately about that. Gavin On Sun, Jan 5, 2014 at 9:34 PM, Nicholas Nethercote <n.netherc...@gmail.com> wrote: > We've had some recent discussions about code style. I have a propasal > > For the purpose of this proposal I will assume that there is consensus on the > following ideas. > > - Having multiple code styles is bad. > > - Therefore, reducing the number of code styles in our code is a win (though > there are some caveats relating to how we get to that state, which I discuss > below). > > - The standard Mozilla style is good enough. (It's not perfect, and it should > continue to evolve, but if you have any pet peeves please mention them in a > different thread to this one.) > > With these ideas in mind, a goal is clear: convert non-Mozilla-style code to > Mozilla-style code, within reason. > > There are two notions that block this goal. > > - Our rule of thumb is to follow existing style in a file. From the style > guide: > > "The following norms should be followed for new code, and for Tower of Babel > code that needs cleanup. For existing code, use the prevailing style in a > file or module, or ask the owner if you are on someone else's turf and it's > not clear what style to use." > > This implies that large-scale changes to convert existing code to standard > style are discouraged. (I'd be interested to hear if people think this > implication is incorrect, though in my experience it is not.) > > I propose that we officially remove this implicit discouragement, and even > encourage changes that convert non-Mozilla-style code to Mozilla-style (with > some exceptions; see below). When modifying badly-styled code, following > existing style is still probably best. > > However, large-scale style fixes have the following downsides. > > - They complicate |hg blame|, but plenty of existing refactorings (e.g. > removing old types) have done likewise, and these are bearable if they > aren't too common. Therefore, style conversions should do entire files in > a single patch, where possible, and such patches should not make any > non-style changes. (However, to ease reviewing, it might be worth > putting fixes to separate style problems in separate patches. E.g. all > indentation fixes could be in one patch, separate from other changes. > These would be combined before landing. See bug 956199 for an example.) > > - They can bitrot patches. This is hard to avoid. > > However, I imagine changes would happen in a piecemeal fashion, e.g. one > module or directory at a time, or even one file at a time. (Again, see bug > 956199 for an example.) A gigantic change-all-the-code patch seems > unrealistic. > > - There is an semi-official policy that the owner of a module can dictate its > style. Examples: SpiderMonkey, Storage, MFBT. > > There appears to be no good reason for this and I propose we remove it. > Possibly with the exception of SpiderMonkey (and XPConnect?), due to it > being > an old and large module with its own well-established style. > > Also, we probably shouldn't change the style of imported third-party code; > even if we aren't tracking upstream, we might still want to trade patches. > (Indeed, it might even be worth having some kind of marking at the top of > files to indicate this, a bit like a modeline?) > > Finally, this is a proposal only to reduce the number of styles in our > codebase. There are other ideas floating around, such as using automated tools > to enforce consistency, but I consider them orthogonal to or > follow-ups/refinements of this proposal -- nothing can happen unless we agree > on a direction (fewer styles!) and a way to move in that direction > (non-trivial > style changes are ok!) > > Nick > _______________________________________________ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
