On Mon, Jul 22, 2019 at 1:20 AM Karl Tomlinson <mozn...@karlt.net> wrote:
> Near the top of > > https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style > there is > > New code should try to conform to these standards, so it is as > > easy to maintain as existing code. [...] > > > > This article is particularly for those new to the Mozilla > > codebase [...]" Before requesting a review, please read over > > this document, making sure that your code conforms to > > recommendations." > > > > Firefox code base uses the Google Coding style for C++ code [2] > > On reading over the document, the reader finds a number of > additional guidelines, some of which are complementary or > orthogonal to Google style and some of which are contradictory. > Yes, that is indeed the case. Sorry about the confusing state of affairs. > Can we clarify which documents have precedence so that we can > spell this out clearly on this page, please? > > "we would like to convert entirely to this [Google C++] coding > style -- perhaps with some exceptions for Mozilla-specific > constraints". [1] > Would the Mozilla coding style be the appropriate place to list > such exceptions? > The existing recording of a recent resolution [3] on this page > seems helpful to me. > I think the best course of action is probably to repurpose this page to describe our current coding style, and move out the documentation for the historical "Mozilla Coding Style" to another page to avoid confusion here. I have a busy schedule in the next couple of days but will do my best to do this reorganization in the next few days. > The reason to think this page might not be the appropriate place > is because [1] > > we will retire the Mozilla C/C++ coding style. The intention > > behind retiring the current Mozilla style is to just stop > > maintaining it as an active coding style going forward, but the > > documentation for this coding style will be kept intact. > > What is the motivation for keeping intact the legacy coding style? > Is it to guide those contributing in code that has not yet moved > to Google style? I wonder how useful that would be given "we now > have significant inconsistencies in the style of our codebase" > [1]? > > If it is useful to keep intact the documentation of legacy style, > can we distinguish which are the guidelines that should apply only > to older code? > The motivation for keeping intact the legacy coding style was to preserve historical documentation in this area (which is still useful when changing existing code), but as I said above I agree that keeping both in the same place isn't ideal. In hindsight we should have split them up into two pages immediately after switching coding styles. > "we are not aiming to enforce any naming conventions at this time" > [1] but both the Google and Mozilla style guides have > (conflicting) naming conventions. Can we clarify which, if any, > naming guidelines apply to new and older code? > FWIW, I suspect that just separating the documentation may not help answer all questions like this. In the case of any possible further confusions, let's have more discussions! If there are areas where we find that what the Google C++ Style says doesn't match our needs I suspect we will end up making small modifications here and there. There is precedent for doing this in large projects that adopt the Google C++ Style (e.g. the Chromium C++ Style https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md ). > The other piece of the equation here is [1] > > our precise coding style will be the result of formatting our > > tree with clang-format, so by definition our code, when > > formatted with clang-format, will be following our new coding > > style. > > clang-format doesn't provide compliance with all guidelines in the > Google style, but it does have precedence, because the only option > to override is `// clang-format off`. clang-format has its own > preferences for formatting, many of which are not documented in > the Google style guide. > Perhaps what that paragraph is trying to say isn't coming across clearly (in which case please suggest a more clear phrasing). The intention behind that paragraph is to tell the reader that if their reason for reading the style guidelines documentation is to figure out how to format whitespace in a specific context, the way we answer that is through running clang-format, rather than following any specific text written somewhere. > Would it be appropriate for the Mozilla coding style page to > indicate that `./mach clang-format` defines the formatting of C++ > code and the Google style guide provides other guidelines, which > should be followed in new code unless there is an exception > listed? > Hmm, I don't think so? To me, whitespace formatting is a subset of coding style matters. For that, we use clang-format. But clang-format doesn't answer all questions regarding our coding style (e.g. how to name things, what types to use, what constructs to use/avoid, etc.) > https://developer.mozilla.org/en-US/docs/Mozilla/Using_CXX_in_Mozilla_code > is another page with more guidelines. I assume we would expect > these to be consistent with those on the Mozilla coding style > page. I assume "Using C++ in Mozilla code" would take precedence > over Google style? > I'm not sure whether the notion of "take precedence" when thinking about these two documents makes sense, and I tend to refrain to answer that question because I haven't thought of all of the possible ways that page and our coding style may interact, so I don't want to give the wrong impression here. But as one of the people who's helped maintain that page over the years, perhaps I can explain what that page has been trying to do. That page has been trying to document the various toolchain support requirements we have around the supported flavour of C++ and its standard library (e.g. C++11, C++14 and so on) and the various sub-features. This document isn't prescriptive but rather descriptive. That is, it tries to tell you whether your code is expected to work if you use feature X on platform Y, not whether you _should_ use feature X. With some C++ features, there can be a great amount of disagreement among various people whether their usage in various parts of our code base should be prescribed/banned. A mere mentioning of those features in the above document is *not* an endorsement of the usage of such features. I hope this helps. Cheers, -- Ehsan _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
