For anyone that missed it, mfinkle's proposal on IRC: <mfinkle> 1. the existing files should not really change their styles <mfinkle> 2. we already have code in the tree (ours and 3rd party) and use a mixture of styles, so keep doing it <mfinkle> 2a. a new platform java file can use the platform style <mfinkle> 2b. a new front-end java file can use the front-end style
On Wed, Apr 23, 2014 at 2:04 PM, Bradford Lassey <[email protected]> wrote: > No, don't do any of this. > > -Brad > > > On 2/27/14, 1:05 PM, Richard Newman wrote: > > 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 <[email protected]> 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" <[email protected]> >> To: [email protected] >> 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 > [email protected] > https://mail.mozilla.org/listinfo/mobile-firefox-dev > > > > _______________________________________________ > mobile-firefox-dev mailing list > [email protected] > https://mail.mozilla.org/listinfo/mobile-firefox-dev > _______________________________________________ mobile-firefox-dev mailing list [email protected] https://mail.mozilla.org/listinfo/mobile-firefox-dev
