Broadening the circulation of this to mobile-firefox-dev. We have approximate consensus within the core frontend team: dropping prefixed names, bracing conditionals, and doing so incrementally as we go.
Ready thyselves! On 27 Feb 2014, at 9:35 AM, Mark Finkle <mfin...@mozilla.com> wrote: > I also agree we should move ahead with the proposed style/structure changes. > > Change: dropping aFoo prefixed arguments, always bracing conditionals, "use > strict" (JS only) > Where/When: New files and piecemeal on edited files. Any changes in an > existing file should be in a separate patch (patch 0). Do not intermingle > substantive and style changes in the same patch. I really do not want to see > bugs filed with patch 0 only. We still need to focus on getting real feature > work completed and not just curb our coding OCD. > > I'm all for the newer style (I've definitely been doing this in my JS), but I > don't want us to spend too much time futzing around updating existing code. > +1 to using the new style in new files, and I'd support "part 0" clean-up > patches in bugs where developers are going to be changing a lot of an > existing file anyway (and that's also a good way to audit the exiting code > for unnoticed problems). > > Margaret > > ----- Original Message ----- > > From: "Richard Newman" <rnew...@mozilla.com> > > To: mobile-front...@mozilla.org > > Sent: Thursday, February 27, 2014 9:01:37 AM > > Subject: On coding standards > > > > Over the past year or three, desktop JS has been moving away from the older > > "toolkit-style" standard: dropping aFoo prefixed arguments, always bracing > > conditionals, "use strict", etc. > > > > We've also done this to an extent in mobile: new Java code tends not to have > > aFoo arguments, and tends not to use unbraced conditionals. > > > > The services code in Fennec (about 1/4 of the codebase) already goes all the > > way, using 'modern' Java coding conventions throughout. > > > > > > I don't expect any pushback on always bracing conditionals, and we're > > already > > dropping aFoo, so: > > > > I propose eliminating the use of mFoo/sBar in the rest of Fennec. Here's > > why. > > > > > > 1. It doesn't add value, when it's wrong it's misleading, and apparently > > nobody pays enough attention to it. For example, GeckoProfile contains this > > line, which wesj wrote in 3c552ee3aa5e, bnicholson reviewed, and nobody > > spotted for at least 6 months: > > > > private static GeckoProfile mGuestProfile = null; > > > > (I'm fixing that as part of Bug 707123.) > > > > If wesj and bnicholson -- two of our most awesome crew -- can make this > > mistake, and nobody spotted it or nobody was affected by it, then why > > bother? > > > > > > 2. It's unnecessarily verbose, considering that it adds no value. At the > > point of definition we already know it's static: it has the word 'static' > > next to it. The best you can do is match that, and the worst is to get it > > wrong! At the point of use, Java already has conventions for being explicit > > about what you're doing: Classname.staticMember, this.member. > > > > > > 3. If you get access wrong, you'll typically get a compiler error. You can't > > access instance members from a static context. You can access statics in > > scope (within a class definition) without qualification, and that's OK. If > > you're worried about getting that wrong, use ClassName.foo instead. The only > > other case that builds -- accessing a static member via an instance -- is > > accompanied by a compiler warning and a yellow flag in Eclipse. > > > > > > 4. Speaking of Eclipse: IDEs make it obvious what everything is, using text > > styling, colors, hints, tooltips, and the other affordances available. I > > acknowledge that this doesn't apply to the process of code review, but I'll > > be so bold as to assert that the set of bugs that might be caught by saying > > "waitaminute, that's static!" is small, uncommon, and probably overlaps with > > the set of bugs that requires you to *understand* the class definition, not > > just pattern-match. > > > > (No disrespect to pattern-match reviewing: I do it all the time.) > > > > > > > > My proposal is to ditch this style for new files, fix up old files as we go > > (just as we do for bracing and broken indents), and enjoy our cheerful new > > world. > > > > Thoughts? > > > > -R
_______________________________________________ mobile-firefox-dev mailing list mobile-firefox-dev@mozilla.org https://mail.mozilla.org/listinfo/mobile-firefox-dev
