Re: On the usefulness of style guides (Was: style guide proposal)

On Thursday 2013-12-19 17:11 -0500, Ehsan Akhgari wrote: > See, that right there is the root problem! Programmers tend to care > too much about their favorite styles. I used to be like that but > over the years I've mostly stopped caring about which style is > better, and what I want now is consi

Re: On the usefulness of style guides (Was: style guide proposal)

On 20/12/13 14:35, Jeff Gilbert 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 soun

Re: On the usefulness of style guides (Was: style guide proposal)

On 12/19/2013, 9:36 PM, Brian Smith wrote: On Thu, Dec 19, 2013 at 6:13 PM, Ehsan Akhgari mailto:ehsan.akhg...@gmail.com>> wrote: But to address the main point of this paragraph, what's wrong with having *one* style that *everybody* follows? I can't tell if you have something agains

Re: On the usefulness of style guides (Was: style guide proposal)

On Thu, Dec 19, 2013 at 6:18 PM, Ehsan Akhgari wrote: > > 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 exist

Re: On the usefulness of style guides (Was: style guide proposal)

On 12/19/2013, 9:40 PM, Nicholas Nethercote wrote: On Thu, Dec 19, 2013 at 6:32 PM, Chris Peterson wrote: I like #3. The common style subset is probably small, but since it is common, it will be uncontroversial and require no bikeshedding. :) I suspect it would be tiny. I recently had to mo

Re: On the usefulness of style guides (Was: style guide proposal)

On Thu, Dec 19, 2013 at 6:32 PM, Chris Peterson wrote: > > I like #3. The common style subset is probably small, but since it is > common, it will be uncontroversial and require no bikeshedding. :) I suspect it would be tiny. I recently had to move a function from SpiderMonkey to Gecko. It was

Re: On the usefulness of style guides (Was: style guide proposal)

On Thu, Dec 19, 2013 at 6:13 PM, Ehsan Akhgari wrote: > But to address the main point of this paragraph, what's wrong with having > *one* style that *everybody* follows? I can't tell if you have something > against that, or if you just care about a small subset of the tree and are > happy with th

Re: On the usefulness of style guides (Was: style guide proposal)

On 12/19/13, 5:35 PM, Jeff Gilbert wrote: 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

Re: On the usefulness of style guides (Was: style guide proposal)

On 12/19/2013, 9:05 PM, Bobby Holley wrote: 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 ch

Re: On the usefulness of style guides (Was: style guide proposal)

On 12/19/2013, 8:35 PM, Jeff Gilbert 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 so

Re: On the usefulness of style guides (Was: style guide proposal)

On Thu, Dec 19, 2013 at 5:35 PM, Jeff Gilbert 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

Re: On the usefulness of style guides (Was: style guide proposal)

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 a

Re: js-inbound as a separate tree

On 19/12/2013 23:56, Jason Orendorff wrote: On 12/19/13 4:55 PM, David Burns wrote: On 19/12/2013 18:48, Jason Orendorff wrote: Con: - more work for sheriffs (mostly merges) If mostly merges, are you suggesting there will be little traffic on the branch or the JS team will watch the tree for f

Re: On the usefulness of style guides (Was: style guide proposal)

I mistakenly sent this only to Jeff: On Thu, Dec 19, 2013 at 6:29 PM, Ehsan Akhgari wrote: > On 12/19/2013, 5:29 PM, Jeff Gilbert wrote: > >> It's compromise. I think we can take it as given that we'll never all >> agree on one way to do things, so instead, different modules do their own >> thing

Re: js-inbound as a separate tree

On 12/19/13 4:55 PM, David Burns wrote: > On 19/12/2013 18:48, Jason Orendorff wrote: >> Con: >> - more work for sheriffs (mostly merges) > > If mostly merges, are you suggesting there will be little traffic on > the branch or the JS team will watch the tree for failures? Neither, I'm just saying

Re: style guide proposal

On 20/12/13 11:55, Mike Hommey wrote: > On Thu, Dec 19, 2013 at 11:02:08AM -0500, Ehsan Akhgari wrote: >> clang-format has a basic support for the Mozilla coding style, and we can >> definitely extend it to add support for this heuristic. > > I don't think we should care about having a Mozilla-sp

Re: On the usefulness of style guides (Was: style guide proposal)

I agree violently with Ehsan and Bobby. Per-module styles are an enormous hassle. (Storage and MFBT are two that come to mind.) Nick ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform

Re: On the usefulness of style guides (Was: style guide proposal)

On 12/20/2013 12:11 AM, Ehsan Akhgari wrote: On 12/19/2013, 12:57 PM, Till Schneidereit wrote: I think we should do more than encourage: we should back out for all style guide violations. Period. We could even enforce that during upload to a review tool, perhaps. However. This has to be done on

Re: js-inbound as a separate tree

On Thu, Dec 19, 2013 at 2:53 PM, Jonathan Griffin wrote: > The disadvantage is that you may end up waiting to have your patch landed, > and some bugs may be hard for the sheriffs to land; e.g., if there are a > lot of patches in a single bug, and some of them have already landed, or > there are pa

Re: On the usefulness of style guides (Was: style guide proposal)

I think that I'm with Ehsan on this one. The value of consistency is such that I think people should be prepared to suppress their inate prejudices in deference to more important goals (wider consistency, better tooling, better productivity). That said, the only time I've ever seen that work i

Re: On the usefulness of style guides (Was: style guide proposal)

On Thu, Dec 19, 2013 at 2:29 PM, Jeff Gilbert 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-

Re: js-inbound as a separate tree

Personally I find the branches we have annoying and are papering over the real problem that our feedback cycles once landed are far too long. Just for that reason alone I am against the idea. I think if we can solve the build/test scheduling and being smart about how we do our testing we can r

Re: style guide proposal

On Thu, Dec 19, 2013 at 11:02:08AM -0500, Ehsan Akhgari wrote: > > How about we just use clang-format? > > http://www.irill.org/videos/euro-llvm-2013/jasper-hires.webm > > > clang-format has a basic support for the Mozilla coding style, and we can > definitely extend it to add support for this h

Re: js-inbound as a separate tree

We already have the approximate equivalent of this. It's the 'checkin-needed' keyword. Add this to your bug, and the sheriffs will land the patch for you, using the approximate process you describe. The only difference is this is done out-of-band, so turnaround may take up to 24 hrs. The a

Re: js-inbound as a separate tree

As someone who works mostly on the intersection of the JS engine and everything else, I'm not really wild about this. SpiderMonkey is pretty intimately tied to the rest of Gecko, certainly just as much as something like gfx. I think fx-team makes more sense, since most of the patches there consist

Re: On the usefulness of style guides (Was: style guide proposal)

It's compromise. I think we can take it as given that we'll never all agree on one way to do things, so instead, different modules do their own things. I think this is a pretty decent solution, though the problem becomes needing different guides for different modules. (the "just mime what you se

Re: On the usefulness of style guides (Was: style guide proposal)

On 12/19/2013, 12:57 PM, Till Schneidereit wrote: I think we should do more than encourage: we should back out for all style guide violations. Period. We could even enforce that during upload to a review tool, perhaps. However. This has to be done on a per-module basis (or even more fine-grained

Re: Should we build a new in-process unwind library?

Here's a status update. Recap: I proposed to create a new library to do fast in process CFI and EXIDX based stack unwinding, so as to be able to profile on tablets and phones with less overhead than using Breakpad. The library now exists. Tracking bug is 938157. It does CFI unwinding on x86_64

js-inbound as a separate tree

On dev-tech-js-engine-internals, there's been some discussion about reviving a separate tree for JS engine development. The tradeoffs are like any other team-specific tree. Pro: - protect the rest of the project from closures and breakage due to JS patches - protect the JS team from closures and

Re: On the usefulness of style guides (Was: style guide proposal)

I think we should do more than encourage: we should back out for all style guide violations. Period. We could even enforce that during upload to a review tool, perhaps. However. This has to be done on a per-module basis (or even more fine-grained: different parts of, e.g., SpiderMonkey have slight

Re: On the usefulness of style guides (Was: style guide proposal)

No question. But there are ways to encourage people to stop digging in. If you provide tools to help those who want to come into the light, say a moz.build option that autoformatted or checked against the general formatting rules, then that might be sufficient. I think that bug 939350 shows a

On the usefulness of style guides (Was: style guide proposal)

Attempting to fork the thread. Please reply here if you want to bikeshed on this topic in general. On Thu, Dec 19, 2013 at 9:38 AM, Martin Thomson wrote: > Here's what I've done for the last few projects I've been on: > > * Taken the off-the-shelf formatter. > * Taken the default configurati

Re: [JS-internals] Improvements to the Sphinx in-tree documentation generation

Moving back to dev-platform. On 12/17/13 4:41 PM, Gregory Szorc wrote: > I guess what I'm trying to say is that SpiderMonkey/JavaScript appears > lacking in the "language services" arena. The concrete features you mentioned are extracting doc comments and minimization, so that's what I'll address

Re: style guide proposal

Here's what I've done for the last few projects I've been on: * Taken the off-the-shelf formatter. * Taken the default configuration. * Applied that frequently. * Moved on to more important things. I understand the desire to come to some sort of consensus on what code should look like. It's

Re: style guide proposal

Can we fork the discussion about style in general into a different thread, so that it doesn't drown out Andrea's proposal? On Thu, Dec 19, 2013 at 9:28 AM, Gregory Szorc wrote: > We have an in-tree Clang plugin in build/clang-plugin that can be used to > deploy more in-depth checking on top of

Re: style guide proposal

We have an in-tree Clang plugin in build/clang-plugin that can be used to deploy more in-depth checking on top of the AST. You can have it emit compiler warnings or errors. It runs as part of automation, so if you write a checker, the tree will burn and bad commits will get backed out. A proble

Re: style guide proposal

Off-the-shelf automated tools are not going to solve our formatting woes. Both the JS engine and Gecko mandate very particular patterns. I ended up writing a custom python tool to reformat XPConnect a few years ago [1] because none of the tools were configurable enough. I think detailed style guid

Re: style guide proposal

If we are talking about checking (a separate thing, in my opinion), then we should be talking about linting :) Checking for formatting is probably less useful than something more concrete. Having run jshint over some of the code, I was horrified at the output it produced. That said, checking

Re: style guide proposal

On Thu, Dec 19, 2013 at 5:02 PM, Ehsan Akhgari wrote: > -- > Ehsan > > > > On Thu, Dec 19, 2013 at 4:32 AM, Mike Hommey wrote: > > How about we just use clang-format? > > http://www.irill.org/videos/euro-llvm-2013/jasper-hires.webm > > > clang-format has a basic suppor

Re: style guide proposal

Funny you should mention that. I was wondering why we didn't have something like this already. For JS: https://github.com/einars/js-beautify (I originally though astyle would be a good choice, but clang-format looks dandy.) - Original Message - From: "David Rajchenbach-Teller" To: "M

Re: style guide proposal

-- Ehsan On Thu, Dec 19, 2013 at 4:32 AM, Mike Hommey wrote: > On Thu, Dec 19, 2013 at 01:20:41AM -0800, Andrea Marchesini wrote: > > Hi, > > > > Writing a patch for bug 949488 I had to indent this piece of code: > > > > nsIScriptSecurityManager* ssm = nsContentUtil

Re: style guide proposal

I'd love that. Also, something similar for JS would be nice. Cheers, David On 12/19/13 10:32 AM, Mike Hommey wrote: > How about we just use clang-format? > http://www.irill.org/videos/euro-llvm-2013/jasper-hires.webm > > Mike > ___ > dev-platform mail

Re: style guide proposal

On 12/19/2013 10:20 AM, Andrea Marchesini wrote: Hi, Writing a patch for bug 949488 I had to indent this piece of code: nsIScriptSecurityManager* ssm = nsContentUtils::GetSecurityManager(); if (NS_WARN_IF(NS_FAILED(ssm->GetSimpleCodebasePrincipal( o

Re: style guide proposal

On Thu, Dec 19, 2013 at 01:20:41AM -0800, Andrea Marchesini wrote: > Hi, > > Writing a patch for bug 949488 I had to indent this piece of code: > > nsIScriptSecurityManager* ssm = nsContentUtils::GetSecurityManager(); > if (NS_WARN_IF(NS_FAILED(ssm->GetSimpleCodebasePrincipal( >

style guide proposal

Hi, Writing a patch for bug 949488 I had to indent this piece of code: nsIScriptSecurityManager* ssm = nsContentUtils::GetSecurityManager(); if (NS_WARN_IF(NS_FAILED(ssm->GetSimpleCodebasePrincipal( originURI, gett