On Thu, Dec 19, 2013 at 6:13 PM, Ehsan Akhgari <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 > against that, or if you just care about a small subset of the tree and are > happy with the status quo in that subset. I've asked people in PSM land to follow the Mozilla Coding Style for new code, except for modifications to pre-existing code. When a group of new people started working on PSM, I took some time to make a widespread change throughout many parts of PSM to make it more consistent, coding-style-wise. However, there are a bunch of rules that I did not enforce as part of that change. I've been tempted to make another mass change to do so, and I am open to other people submitting patches in my module to make the code more consistent with the Mozilla coding style. As a Necko peer, I would welcome such changes in Necko too. However, it would be great to agree on what changes are going to be done, before a large amount of effort is spent doing them. I don't think that everybody is as agreeable as me though. When I've been asked to review code in other modules, my attempts to get people to follow the Mozilla coding style, instead of the module peers'/owners' style, received a lot of pushback. WebRTC comes to mind, though I think the pushback was probably more over concern for delaying the landing of the feature over concern about style. > 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. >> > > That is not very helpful. If there is something in the mozilla style > guide that you think is wrong and needs to change, *please* bring it up. > If you're right, you'll be benefiting everyone. And if it's just a matter > of taste, perhaps you could sacrifice your preferences in the interest of > the greater good? In PSM, we created some scoped pointer wrapper types around NSS data structures (ScopedNSSTypes.h), which are based on the MFBT scoped pointers. And, consequently, PSM has standardized on MFBT smart pointers throughout the module (there should be nsRefPtr in PSM, only RefPtr, for example). Yet, most code in Gecko is based on the nsCOMPtr-like smart pointers (nsAutoPtr, nsRefPtr). I don't know how big of a deal this is, but this is the type of thing that would need to be resolved to have a consistent style throughout Gecko. > > It's a decent amount of work to restyle the modules well > > That's actually not true. There are tools which are very good at doing > this work once we agree that it should be done. Color me skeptical. I wouldn't want somebody to reformat the code in the modules I have responsibility for without reviewing the changes. And, reviewing tens of thousands of lines of changes is a lot of work. > I don't think that anybody is suggesting that we come up with a set of > style guides and carve them into stone and never consider anything > otherwise. But then again debating where the * in a pointer notation ends > up with every week isn't the best use of everybody's time. If and when > someone finds something wrong in the style guideline they can bring it up > and get the style modified if they have a good point. Note that this is > quite doable, as evidence of other projects which do this well shows. If somebody submitted a patch to fix the "*" issue throughout PSM, I would r+ it, though I don't look forward to spending the time to do it, especially considering the issue of bitrot. (Please do not write such a patch before the end of January; it wouldn't get r+d before then, due to the bitrot issue with pending work.) 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. I agree. With two exceptions, everything style-related related seems to be insignificant regarding how much time I spend on stuff. I just make my code look like the code around it, and if the reviewer complains about style issues I generally just do whatever the reviewer wants so I don't need to argue back-and-forth. Very simple. However, there are two issues that are non-trivial distractions from real work: 1. Many parts of NSS use tabs instead of spaces. AFAICT, this is an issue for which the idea of fixing things is more-or-less agreed upon. But, no time to actually do it. 2. Not everybody succeeds at making their new code look like the code around it (or, in some cases, like any other code on Earth). I (and others) waste a large amount of time during code reviews pointing out style nits. If there were a tool that people were required to run to self-review their code before asking for review, and that tool required work to make our code more consistent in order for it to work correctly, I would be more enthusiastic about taking time to deal with the inconvenience of reformatting the parts of the codebase I work on (NSPR and NSS aside, since other than replacing tabs with spaces, it is unlikely to get agreement from the other peers/owners to make wholesale style changes in those modules.) Cheers, Brian -- Mozilla Networking/Crypto/Security (Necko/NSS/PSM) _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
