On 01/06/2014 08:06 PM, Bobby Holley wrote:
I think trying to change SpiderMonkey style to Gecko isn't a great use of
effort:
* There are a lot of SpiderMonkey hackers who don't hack on anything else,
and don't consider themselves Gecko hackers. Many of them don't read
dev-platform, and would probably revolt if this were to occur.
* SpiderMonkey is kind of an external module, with an existing community of
embedders who use its style.
* SpiderMonkey has an incredibly detailed style guide - more detailed than
Gecko's: https://wiki.mozilla.org/JavaScript:SpiderMonkey:Coding_Style
I don't see that being any more detailed than Mozilla coding style.
Perhaps more verbal, but not more detailed.
* The stylistic consistency and attention to detail is probably the highest
out of any other similarly-sized module in the tree.
This leaves us with the question about what to do with XPConnect. It used
to have its own (awful) style, and I converted it to SpiderMonkey style 2
years ago (at the then-module-owner's request). It interacts heavily with
the JS engine, so this sorta makes sense, but there's also an argument for
converting it to Gecko style. I could perhaps be persuaded at some point if
someone wants to do the leg work.
bholley
On Mon, Jan 6, 2014 at 6:07 AM, smaug <sm...@welho.com> wrote:
Sounds good, and I'd include also js/* so that we had consistent style
everywhere.
It is rather painful to hack various non-js/* and js/* (xpconnect in my
case)
in the same patch.
(I also happen to think that Mozilla coding style is inherently better
than js style, since
it has clear rules for naming parameters and static, global and member
variables in a way
which make them look different to local variables.)
3rd party code shouldn't be touched.
-Olli
On 01/06/2014 04:34 AM, Nicholas Nethercote 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
