Re: List of deprecated constructs [was Re: A proposal to reduce the number of styles in Mozilla code]

2014-01-07 Thread Ms2ger

On 01/07/2014 01:11 AM, Joshua Cranmer  wrote:

On 1/6/2014 6:06 PM, smaug wrote:

On 01/07/2014 01:38 AM, Joshua Cranmer  wrote:

On 1/6/2014 4:27 PM, Robert O'Callahan wrote:

That's just not true, sorry. If some module owner decides to keep
using NULL or PRUnichar, or invent their own string class, they will
be corrected.


Maybe. But we also have a very large number of deprecated or
effectively-deprecated constructs whose deprecation module owners may
not be aware of
because their use is somewhat prevalent in code. For example, the
NS_ENSURE_* macros are apparently now considered officially deprecated.

Since when? NS_ENSURE_ macros are very useful for debugging. (When
something is going wrong, the warnings in the terminal tend to give
strong
hints what/where that something is. Reduces debugging time
significantly.)


Since Benjamin's message of November 22:
news://news.mozilla.org/mailman.11861.1385151580.23840.dev-platf...@lists.mozilla.org
(search for Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS if you
don't have an NNTP client). Although there wasn't any prior discussion
of the wisdom of this change, whether or not to use
NS_ENSURE_SUCCESS-like patterns has been the subject of very acrimonious
debates in the past and given the voluminous discussion on style guides
in recent times, I'm not particularly inclined to start yet another one.


FWIW, I've never seen much support for this change from anyone else than 
Benjamin, and only in his modules the NS_ENSURE_* macros have been 
effectively deprecated.


HTH
Ms2ger

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Ms2ger

On 01/07/2014 01:46 AM, Jeff Walden wrote:

I don't think most JS hackers care for abuse of Hungarian notation
for scope-based (or const) naming.  Every member/argument having a
capital letter in it surely makes typing slower.  And extra noise in
every name but locals seems worse for new-contributor readability.
Personally this doesn't bother me much (although aCx will always be
painful compared to cx as two no-cap letters, I'm sure), but others
are much more bothered.


Based on the discussion in #jsapi yesterday, I'm not sure that most JS 
hackers is necessarily accurate.


HTH
Ms2ger


___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: List of deprecated constructs [was Re: A proposal to reduce the number of styles in Mozilla code]

2014-01-07 Thread L. David Baron
On Tuesday 2014-01-07 09:13 +0100, Ms2ger wrote:
 On 01/07/2014 01:11 AM, Joshua Cranmer  wrote:
 Since Benjamin's message of November 22:
 news://news.mozilla.org/mailman.11861.1385151580.23840.dev-platf...@lists.mozilla.org
 (search for Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS if you
 don't have an NNTP client). Although there wasn't any prior discussion
 of the wisdom of this change, whether or not to use
 NS_ENSURE_SUCCESS-like patterns has been the subject of very acrimonious
 debates in the past and given the voluminous discussion on style guides
 in recent times, I'm not particularly inclined to start yet another one.
 
 FWIW, I've never seen much support for this change from anyone else
 than Benjamin, and only in his modules the NS_ENSURE_* macros have
 been effectively deprecated.

I'm happy about getting rid of NS_ENSURE_*.

-David

-- 
턞   L. David Baron http://dbaron.org/   턂
턢   Mozilla  https://www.mozilla.org/   턂
 Before I built a wall I'd ask to know
 What I was walling in or walling out,
 And to whom I was like to give offense.
   - Robert Frost, Mending Wall (1914)


signature.asc
Description: Digital signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Jason Duell

On 01/06/2014 06:35 PM, Joshua Cranmer  wrote:


Side-by-side diffs are one use case I can think of. Another is that some
people prefer to develop by keeping tiled copies of windows; wider lines
reduce the number of tiled windows that one can see.


Yes--if we jump to 80 chars per line, I won't be able to keep two 
columns open in my editor (vim, but emacs would be the same) on my 
laptop, which would suck.


(Yes, my vision is not what it used to be--I'm using 10 point font. But 
that's not so huge.)


Jason



People who use

terminal-based editors for their coding are probably going to be rather
likely to use the default window size for terminals: 80x24. Given that
our style guide also requires adding vim and emacs modelines to files
(aside: if we're talking about doing mass style-conversions, can we also
fix modelines?), it seems reasonable that enough of our developers use
vim and emacs to make force-resizing of terminal size defaults a
noticeable cost.

With 2-space indent, parsimonious indenting requirements (e.g., don't
indent case: statements in switch or public in class), and liberal use
of importing names into localized namespaces, the 80-column width isn't
a big deal for most code.

I don't think most JS hackers care for abuse of Hungarian notation for
scope-based (or const) naming.  Every member/argument having a capital
letter in it surely makes typing slower.  And extra noise in every
name but locals seems worse for new-contributor readability.
Personally this doesn't bother me much (although aCx will always be
painful compared to cx as two no-cap letters, I'm sure), but others
are much more bothered.


And a '_' at the end of member names requires less typing than 'm' +
capital letter?

My choice of when to use or not use Hungarian notation is often messy
and seemingly inconsistent, although there is some method to my madness.
I find prefixing member variables with 'm' to be useful, although I
dislike using it in POD-ish structs where all the members are public.
The use of 'a' for arguments is where I am least consistent, especially
as I extremely dislike it being used for an outparam return value
(disclaimer: I'm used to XPCOM-taminated code, so having to write
NS_IMETHODIMP nsFoo::GetBoolValue(bool *retval) is common for me, and
this colors my judgements a lot). I've never found much use for the 's',
'g', and 'k' prefixes, although that may just as well be because I've
never found much use for using those types of variables in the first
place (or when I do, it's because I'm being dictated by other concerns
instead, e.g., type traits-like coding or C++11 polyfilling).



___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS

2014-01-07 Thread Karl Tomlinson
Bobby Holley writes:

 Note that there in a explicit stylistic exception that NS_WARN_IF
 statements do not require braces. So it's:

 if (NS_WARN_IF(NS_FAILED(rv)))
   return rv;

I don't see that on the current version of
https://developer.mozilla.org/en-US/docs/Developer_Guide/Coding_Style

I like having entirely blank lines (without braces) after jump
statements because I find it helps make them stand out, but I
don't see any reason why there should be an exception only for
particular macros.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Bug 641509

2014-01-07 Thread matteosistisette
On https://bugzilla.mozilla.org/show_bug.cgi?id=641509 in the comments I can't 
see any valid argument that backs up the decision to not fix the issue.
At least none that would stand to the objection which I posted as a comment:

 Having a standard message always displayed is ok, 
 but what's the reasoning behind not allowing to _add_ a custom text?!?!?!?

In reply to my comment the issue was closed to comments.

Instead of that, could anybody reply with a reasonable argument and prove my 
reasoning wrong? (I put reasoning between quotes because I find it almost 
too elementary and obvious to be even called a reasoning)

Can anybody explain not only to me but also to the number of people who 
questioned the same, why the decision to not fix the issue is a good idea? 
(despite by the way every other browser supporting that, except Opera which 
does not support the onbeforeunload event in the first place)
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Bug 641509

2014-01-07 Thread Anne van Kesteren
On Tue, Jan 7, 2014 at 11:39 AM,  matteosistise...@gmail.com wrote:
 Can anybody explain not only to me but also to the number of people who 
 questioned the same, why the decision to not fix the issue is a good idea? 
 (despite by the way every other browser supporting that, except Opera which 
 does not support the onbeforeunload event in the first place)

We support the event, we do not support modifying the message shown to
the user to prevent spoofing, as explained in the bug. This is
perfectly acceptable per the standard that defines the event:
http://www.whatwg.org/specs/web-apps/current-work/#prompt-to-unload-a-document


-- 
http://annevankesteren.nl/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS

2014-01-07 Thread smaug

On 01/07/2014 05:14 PM, smaug wrote:

On 01/07/2014 08:46 AM, Bobby Holley wrote:

On Mon, Jan 6, 2014 at 5:04 PM, smaug sm...@welho.com wrote:


no, since it is always possible to expand those macros.
However
if (NS_WARN_IF(NS_FAILED(rv)) {
   return rv;
}
is super ugly.



Note that there in a explicit stylistic exception that NS_WARN_IF
statements do not require braces. So it's:

No exceptions. always {} with if.





if (NS_WARN_IF(NS_FAILED(rv)))
   return rv;


Also, I agree that we should get NS_WARN_IF_FAILED. Then it becomes:

if (NS_WARN_IF_FAILED(rv))
   return rv;

which is almost as palatable as NS_ENSURE_SUCCESS(rv, rv);

bholley






And looks like whoever updated the coding style made the examples inconsistent, 
since there has pretty much always
(I think since the time coding style was somewhere in www.mozilla.org/*) been the 
following Always brace controlled statements, and
that is still there. No exceptions. Consistency is more important than style.




___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS

2014-01-07 Thread smaug

On 01/07/2014 08:46 AM, Bobby Holley wrote:

On Mon, Jan 6, 2014 at 5:04 PM, smaug sm...@welho.com wrote:


no, since it is always possible to expand those macros.
However
if (NS_WARN_IF(NS_FAILED(rv)) {
   return rv;
}
is super ugly.



Note that there in a explicit stylistic exception that NS_WARN_IF
statements do not require braces. So it's:

No exceptions. always {} with if.





if (NS_WARN_IF(NS_FAILED(rv)))
   return rv;


Also, I agree that we should get NS_WARN_IF_FAILED. Then it becomes:

if (NS_WARN_IF_FAILED(rv))
   return rv;

which is almost as palatable as NS_ENSURE_SUCCESS(rv, rv);

bholley




___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Gervase Markham
On 07/01/14 00:46, Jeff Walden wrote:
 JS widely uses 99ch line lengths (allows a line-wrap character in
 100ch terminals).  Given C++ symbol names, especially with templates,
 get pretty long, it's a huge loss to revert to 80ch because of how
 much has to wrap.  Is there a reason Mozilla couldn't increase to 99
 or 100?  Viewability on-screen seems pretty weak in this era of
 generally large screens.  Printability's a better argument, but it's
 unclear to me files are printed often enough for this to matter.  I
 do it one or two times a year, myself, these days.
 
 I don't think most JS hackers care for abuse of Hungarian notation
 for scope-based (or const) naming.

It would be very interesting for someone to see if any of the references
Mike Hoye gives explain _which_ types of change lead to loss of
productivity. For example, it could be that brace style does, and line
length does not.

Gerv
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: A proposal to reduce the number of styles in Mozilla code

2014-01-07 Thread Mike Hoye

On 1/6/2014, 8:05 PM, Karl Tomlinson wrote:

The bug is hidden almost as well by conversion of only indentation.

   if (condition1)
 if (condition2)
   foo();
 else
   bar();

Sure, you could argue that style conversion makes the actual
behaviour clearer, but you'd have to know the intended behaviour
to know there was a bug.

If we have a tool to skip the style change on any such unclear
situations, then perhaps we can proceed more safely.


A syntactic change that's small, easy to review and limited to one part 
of one file, you say?


So as you describe it, a tool that backs off and flags ambiguities like 
this for human review would also be a tool that generates an _excellent_ 
set of Good First Bugs.



- mhoye
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread jeffshields8
On Monday, January 6, 2014 6:46:49 PM UTC-6, Jeff Walden wrote:
 I'm writing this list, so obviously I'm choosing what I think is on it.  But 
 I think there's rough consensus on most of these among JS hackers.
 
 
 
 JS widely uses 99ch line lengths (allows a line-wrap character in 100ch 
 terminals).  Given C++ symbol names, especially with templates, get pretty 
 long, it's a huge loss to revert to 80ch because of how much has to wrap.  Is 
 there a reason Mozilla couldn't increase to 99 or 100?  Viewability on-screen 
 seems pretty weak in this era of generally large screens.  Printability's a 
 better argument, but it's unclear to me files are printed often enough for 
 this to matter.  I do it one or two times a year, myself, these days.
 
 
 
 I don't think most JS hackers care for abuse of Hungarian notation for 
 scope-based (or const) naming.  Every member/argument having a capital letter 
 in it surely makes typing slower.  And extra noise in every name but locals 
 seems worse for new-contributor readability.  Personally this doesn't bother 
 me much (although aCx will always be painful compared to cx as two no-cap 
 letters, I'm sure), but others are much more bothered.
 
 
 
 JS people have long worked without bracing single-liners.  With any style 
 guide's indentation requirements, they're a visually redundant waste of 
 space.  Any style checker that checks both indentation and bracing (of course 
 we'll have one, right?), will warn twice for the error single-line bracing 
 prevents.  I think most of us would discount the value of being able to add 
 more to a single-line block without changing the condition line.  So I'm 
 pretty sure we're all dim on this one.
 
 
 
 Skimming the rest of the current list, I don't see anything that would 
 obviously, definitely, be on the short list of complaints for SpiderMonkey 
 hackers.  Other SpiderMonkey hackers should feel free to point out anything 
 else they see, that I might have missed.
 
 
 
 Jeff francis shields

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: A proposal to reduce the number of styles in Mozilla code

2014-01-07 Thread Ehsan Akhgari

On 1/6/2014, 8:05 PM, Karl Tomlinson wrote:

Gregory Szorc writes:


I think you just gave an example of why our tooling needs to
identify poorly formatted code better. An if without a brace
followed by multiple indented lines containing compiler
expressions is a bug at worst or unreadable/unclear code at
best. We could, of course, identify such poor style before any
automated style conversion. In any case, looking at a file
revision from before the automated brace insertion would clearly
reveal the author's [likely] intent. I therefore fail to see your
concern here.


Consider

 if (condition1)
 if (condition2)
 foo();
 else
 bar();

Automated style conversion would make this

   if (condition1) {
 if (condition2) {
   foo();
 } else {
   bar();
 }
   }

No one is likely to look at the file revision before the style
conversion, but someone might look through the trunk revision.
The bug is initially visible but well hidden by the conversion.

The bug is hidden almost as well by conversion of only indentation.

   if (condition1)
 if (condition2)
   foo();
 else
   bar();

Sure, you could argue that style conversion makes the actual
behaviour clearer, but you'd have to know the intended behaviour
to know there was a bug.

If we have a tool to skip the style change on any such unclear
situations, then perhaps we can proceed more safely.


clang-format does not yet support automatic brace insertion.  Once that 
support is added, we can protect against these cases.


Cheers,
Ehsan

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Bug 641509

2014-01-07 Thread Till Schneidereit
My point is that your arguments have been brought forward in the
discussion, and your post didn't bring anything new to the table. You might
disagree with the decision the contributors and owners of the affected
module have come to, and that's your right.

Snide remarks and shooting down exaggerated strawman versions of their
arguments, however, won't help you reverse this decision. If anything, a
well thought-out argument put forward in a reasonable way would. Keep in
mind that this argument should contain something not yet mentioned in the
discussion. Otherwise it has very slim chances of reverting a decision that
stood regardless of previous counterarguments for years now.


On Tue, Jan 7, 2014 at 5:40 PM, Matteo Sisti Sette 
matteosistise...@gmail.com wrote:

 I did read the discussion in the bug I commented on (not the other one I
 admit) and the security reasons argument is flawed in that it assumes
 that the author-provided text would REPLACE the standard warning text,
 while in fact the requested fix (and what EVERY other browser does) is to
 ADD the text provided by the site to the standard warning provided by the
 browser. How can that possibly be a security concern?


People disagree with your assessment, clearly.


 Oh now I see, 'the *confusion* of the OMG YOUR COMPUTER IS INFECTED
 BY A VIRUS messages were causing', that's the point! LOL

 Then based on that we shouldn't allow the browser to open a web page at
 all, because it could contain that kind of message. Or, to be a little less
 extreme and sarchastic, we should definitely disallow arert() based on that
 exact same argument.
 Note, by the way, that alert() _had_ been a security concern in the old
 days and a solution was sought and found, without having to drop the
 feature completely. Think about that.


As pointed out above, your chances of getting people to actually think
about the merits of what you're saying would be vastly higher if you didn't
ridicule them or sarcastically misrepresenting their arguments.


 By the way I see the behavior every other browser implements described
 here:
 http://www.whatwg.org/specs/web-apps/current-work/multipage/webappapis.html
 Not sure but that may mean that perhaps it might some day end up being
 included in the standards.


Step 8 of what Anne linked to earlier explicitly specifies that displaying
a custom message is optional: The prompt shown by the user agent may
include the string of the returnValue attribute, or some leading subset
thereof. (A user agent may want to truncate the string to 1024 characters
for display, for instance.)
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Jeff Walden
On 01/07/2014 02:20 AM, Ms2ger wrote:
 Based on the discussion in #jsapi yesterday, I'm not sure that most JS 
 hackers is necessarily accurate.

I think there's a rough consensus.  And I'd note only a few of us were really 
active in that conversation, and I'm going somewhat off memory of what others 
have said in the past.

Jeff
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Adam Roach

On 1/7/14 03:07, Jason Duell wrote:
Yes--if we jump to 80 chars per line, I won't be able to keep two 
columns open in my editor (vim, but emacs would be the same) on my 
laptop, which would suck.


(Yes, my vision is not what it used to be--I'm using 10 point font. 
But that's not so huge.) 


I'm not just sympathetic to this argument; I've made it myself in other 
venues. Put me down as emphatically agreeing.



--
Adam Roach
Principal Platform Engineer
a...@mozilla.com
+1 650 903 0800 x863
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: A proposal to reduce the number of styles in Mozilla code

2014-01-07 Thread Boris Zbarsky

On 1/7/14 12:47 PM, Martin Thomson wrote:

I’ll just point out that this isn’t a bug if there are test cases that pass.


That depends on whether the test cases are correct (e.g. whether they 
were written independently of the code, whether they were reviewed, etc)


-Boris

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Bobby Holley
On Tue, Jan 7, 2014 at 9:38 AM, Adam Roach a...@mozilla.com wrote:

 On 1/7/14 03:07, Jason Duell wrote:

 Yes--if we jump to 80 chars per line, I won't be able to keep two
 columns open in my editor (vim, but emacs would be the same) on my laptop,
 which would suck.

 (Yes, my vision is not what it used to be--I'm using 10 point font. But
 that's not so huge.)


 I'm not just sympathetic to this argument; I've made it myself in other
 venues. Put me down as emphatically agreeing.


Me too. With the font size that works for me, I can do 2 side-by-side
columns with 80 chars, but not 100.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS

2014-01-07 Thread Andrew McCreight
I filed bug 957201 for NS_WARN_IF_FAILED.

Andrew

- Original Message -
 On 01/07/2014 02:58 AM, Karl Tomlinson wrote:
  smaug sm...@welho.com writes:
 
  Why this deprecation?
 
  NS_ENSURE_ macros hid return paths.
  Also many people didn't understand that they issued warnings, and
  so used the macros for expected return paths.
 
  Was there some useful functionality that is not provided by the
  replacements?
 
 
 
 no, since it is always possible to expand those macros.
 However
 if (NS_WARN_IF(NS_FAILED(rv)) {
return rv;
 }
 is super ugly.
 
 Hopefully something like NS_WARN_IF_FAILED(rv) could be added.
 ___
 dev-platform mailing list
 dev-platform@lists.mozilla.org
 https://lists.mozilla.org/listinfo/dev-platform
 
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread L. David Baron
On Tuesday 2014-01-07 10:23 -0800, Bobby Holley wrote:
 On Tue, Jan 7, 2014 at 9:38 AM, Adam Roach a...@mozilla.com wrote:
 
  On 1/7/14 03:07, Jason Duell wrote:
 
  Yes--if we jump to 80 chars per line, I won't be able to keep two
  columns open in my editor (vim, but emacs would be the same) on my laptop,
  which would suck.
 
  (Yes, my vision is not what it used to be--I'm using 10 point font. But
  that's not so huge.)
 
 
  I'm not just sympathetic to this argument; I've made it myself in other
  venues. Put me down as emphatically agreeing.
 
 
 Me too. With the font size that works for me, I can do 2 side-by-side
 columns with 80 chars, but not 100.

Same for me, for my laptop display.

On my external monitors (office and home; different sizes), I can
fit two at 100, but only barely so for my external monitor at home,
and it might not stay that way the next time the available fonts
change or font rasterization changes and I have to reconfigure.

(The reality for me is that I don't use side-by-side much when
writing code, but I do when doing code reviews.)

-David

-- 
턞   L. David Baron http://dbaron.org/   턂
턢   Mozilla  https://www.mozilla.org/   턂
 Before I built a wall I'd ask to know
 What I was walling in or walling out,
 And to whom I was like to give offense.
   - Robert Frost, Mending Wall (1914)


signature.asc
Description: Digital signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS

2014-01-07 Thread Bobby Holley
Hm. It's pretty unfortunate that we now need 4 lines per fallible call, as
opposed to 2 with NS_ENSURE_* macros. The stylistic exception made this 3,
which was slightly more palatable.

bholley


On Tue, Jan 7, 2014 at 7:26 AM, smaug sm...@welho.com wrote:

 On 01/07/2014 05:14 PM, smaug wrote:

 On 01/07/2014 08:46 AM, Bobby Holley wrote:

 On Mon, Jan 6, 2014 at 5:04 PM, smaug sm...@welho.com wrote:

  no, since it is always possible to expand those macros.
 However
 if (NS_WARN_IF(NS_FAILED(rv)) {
return rv;
 }
 is super ugly.


 Note that there in a explicit stylistic exception that NS_WARN_IF
 statements do not require braces. So it's:

 No exceptions. always {} with if.




 if (NS_WARN_IF(NS_FAILED(rv)))
return rv;


 Also, I agree that we should get NS_WARN_IF_FAILED. Then it becomes:

 if (NS_WARN_IF_FAILED(rv))
return rv;

 which is almost as palatable as NS_ENSURE_SUCCESS(rv, rv);

 bholley




 And looks like whoever updated the coding style made the examples
 inconsistent, since there has pretty much always
 (I think since the time coding style was somewhere in www.mozilla.org/*)
 been the following Always brace controlled statements, and
 that is still there. No exceptions. Consistency is more important than
 style.





 ___
 dev-platform mailing list
 dev-platform@lists.mozilla.org
 https://lists.mozilla.org/listinfo/dev-platform

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: List of deprecated constructs [was Re: A proposal to reduce the number of styles in Mozilla code]

2014-01-07 Thread Benoit Jacob
2014/1/7 L. David Baron dba...@dbaron.org

 On Tuesday 2014-01-07 09:13 +0100, Ms2ger wrote:
  On 01/07/2014 01:11 AM, Joshua Cranmer  wrote:
  Since Benjamin's message of November 22:
  news://
 news.mozilla.org/mailman.11861.1385151580.23840.dev-platf...@lists.mozilla.org
 
  (search for Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS if you
  don't have an NNTP client). Although there wasn't any prior discussion
  of the wisdom of this change, whether or not to use
  NS_ENSURE_SUCCESS-like patterns has been the subject of very acrimonious
  debates in the past and given the voluminous discussion on style guides
  in recent times, I'm not particularly inclined to start yet another one.
 
  FWIW, I've never seen much support for this change from anyone else
  than Benjamin, and only in his modules the NS_ENSURE_* macros have
  been effectively deprecated.

 I'm happy about getting rid of NS_ENSURE_*.


I would like a random voice in favor of deprecating NS_ENSURE_* for the
reason that hiding control flow behind macros is IMO one of the most evil
usage patterns of macros.

Benoit



 -David

 --
 턞   L. David Baron http://dbaron.org/   턂
 턢   Mozilla  https://www.mozilla.org/   턂
  Before I built a wall I'd ask to know
  What I was walling in or walling out,
  And to whom I was like to give offense.
- Robert Frost, Mending Wall (1914)

 ___
 dev-platform mailing list
 dev-platform@lists.mozilla.org
 https://lists.mozilla.org/listinfo/dev-platform


___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: List of deprecated constructs [was Re: A proposal to reduce the number of styles in Mozilla code]

2014-01-07 Thread Gavin Sharp
I support getting rid of NS_ENSURE_*.

Gavin

On Tue, Jan 7, 2014 at 3:13 AM, Ms2ger ms2...@gmail.com wrote:
 On 01/07/2014 01:11 AM, Joshua Cranmer  wrote:

 On 1/6/2014 6:06 PM, smaug wrote:

 On 01/07/2014 01:38 AM, Joshua Cranmer  wrote:

 On 1/6/2014 4:27 PM, Robert O'Callahan wrote:

 That's just not true, sorry. If some module owner decides to keep
 using NULL or PRUnichar, or invent their own string class, they will
 be corrected.


 Maybe. But we also have a very large number of deprecated or
 effectively-deprecated constructs whose deprecation module owners may
 not be aware of
 because their use is somewhat prevalent in code. For example, the
 NS_ENSURE_* macros are apparently now considered officially deprecated.

 Since when? NS_ENSURE_ macros are very useful for debugging. (When
 something is going wrong, the warnings in the terminal tend to give
 strong
 hints what/where that something is. Reduces debugging time
 significantly.)


 Since Benjamin's message of November 22:

 news://news.mozilla.org/mailman.11861.1385151580.23840.dev-platf...@lists.mozilla.org
 (search for Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS if you
 don't have an NNTP client). Although there wasn't any prior discussion
 of the wisdom of this change, whether or not to use
 NS_ENSURE_SUCCESS-like patterns has been the subject of very acrimonious
 debates in the past and given the voluminous discussion on style guides
 in recent times, I'm not particularly inclined to start yet another one.


 FWIW, I've never seen much support for this change from anyone else than
 Benjamin, and only in his modules the NS_ENSURE_* macros have been
 effectively deprecated.

 HTH
 Ms2ger


 ___
 dev-platform mailing list
 dev-platform@lists.mozilla.org
 https://lists.mozilla.org/listinfo/dev-platform
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: List of deprecated constructs [was Re: A proposal to reduce the number of styles in Mozilla code]

2014-01-07 Thread Neil

Benoit Jacob wrote:


I would like a random voice in favor of deprecating NS_ENSURE_* for the reason 
that hiding control flow behind macros is IMO one of the most evil usage 
patterns of macros.


So you're saying that

nsresult rv = Foo();
NS_ENSURE_SUCCESS(rv, rv);

is hiding the control flow of the equivalent JavaScript

try {
   Foo();
} catch (e) {
   throw e;
}

except of course that nobody writes JavaScript like that...

--
Warning: May contain traces of nuts.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Neil

Martin Thomson wrote:


in JS, a case that I’ve encountered several times:
 
 _classMethodName: function ReasonableClassName__classMethodName(argument1, argument2) {


I thought that our debugging story had improved sufficiently that we 
didn't need these fake function names any more.


--
Warning: May contain traces of nuts.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: List of deprecated constructs [was Re: A proposal to reduce the number of styles in Mozilla code]

2014-01-07 Thread Benoit Jacob
2014/1/7 Neil n...@parkwaycc.co.uk

 Benoit Jacob wrote:

  I would like a random voice in favor of deprecating NS_ENSURE_* for the
 reason that hiding control flow behind macros is IMO one of the most evil
 usage patterns of macros.

  So you're saying that

 nsresult rv = Foo();
 NS_ENSURE_SUCCESS(rv, rv);

 is hiding the control flow of the equivalent JavaScript

 try {
Foo();
 } catch (e) {
throw e;
 }

 except of course that nobody writes JavaScript like that...


All I mean is that NS_ENSURE_SUCCESS hides a 'return' statement.

#define NS_ENSURE_SUCCESS(res, ret)
   \  do {
   \nsresult __rv = res; /* Don't evaluate |res| more than
once */\if (NS_FAILED(__rv)) {
   \  NS_ENSURE_SUCCESS_BODY(res, ret)
   \  return ret;
   \}
   \  } while(0)


For example, if I'm scanning a function for possible early returns (say I'm
debugging a bug where we're forgetting to close or delete a thing before
returning), I now need to scan for NS_ENSURE_SUCCESS in addition to
scanning for return. That's why hiding control flow in macros is, in my
opinion, never acceptable.

Benoit



 --
 Warning: May contain traces of nuts.

 ___
 dev-platform mailing list
 dev-platform@lists.mozilla.org
 https://lists.mozilla.org/listinfo/dev-platform

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Mike Hoye

On 1/7/2014, 10:44 AM, Gervase Markham wrote:
  It would be very interesting for someone to see if any of the 
references Mike Hoye gives explain _which_ types of change lead to 
loss of productivity. For example, it could be that brace style does, 
and line length does not. 


I've been digging into the research style and formatting questions a 
bit, and this is what I have so far.


Short version:

- Indent with 4 spaces,
- use CamelCase,
- bracing style doesn't matter,
- consistency is The Most Important Thing.

Those are all consistently-replicated results. I don't have anything 
about line-length yet; I'm working on it.


The details:

On Spacing and indentation: Steve McConnell, in his book Code 
Complete, says that S ubjects scored 20 to 30 percent higher on a test 
of comprehension when programs had a two-to-four-spaces indentation 
scheme than they did when programs had no indentation at all. [...] The 
study concluded that two-to-four-space indentation was optimal. 
Interestingly, many subjects in the experiment felt that the six-space 
indentation was easier to use than the smaller indentations, even though 
their scores were lower. That’s probably because six space indentation 
looks pleasing. But regardless of how pretty it looks, six-space 
indentation turns out to be less readable. This is an example of a 
collision be tween aesthetic appeal and readability.


I've ordered a copy of his book, to figure out what the original source 
material actually says, but my understanding is that Python standardized 
on four-space indentation because of these results.


Brace style:  McConnell suggests - citing the Soloway and Ehrlich paper 
I mentioned, and others, that the specific brace style hasn't been 
demonstrated to matter all that much, provided it is always used 
consistently. Inconsistent bracing (exactly like every other 
inconsistently-applied coding style...) is what sends your thought 
process off into a ditch.


On CamelCase v. underscore_delimited variable naming and comprehension: 
Bonita Sharif and Jonathan Maletic at Kent State have concluded that - 
with some caveats about novice programmers - there's not a big 
difference in comprehension and accuracy between the two. That paper is 
here:


http://www.cs.kent.edu/~jmaletic/papers/ICPC2010-CamelCaseUnderScoreClouds.pdf

However, a related paper available here - 
http://ieeexplore.ieee.org/xpl/freeabs_all.jsp?arnumber=5090039 - (don't 
have the full paper, sorry) concludes that among experienced 
programmers, CamelCase, not under_score,  is measurably the right thing.


So, there you go.


- mhoye



___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Martin Thomson
On 2014-01-07, at 11:28, Benjamin Smedberg benja...@smedbergs.us wrote:

 Especially for new contributors we shouldn't assume that most editors can do 
 this correctly. I personally think that the risk of introducing a new member 
 and then having locals shadow members is real enough that we should somehow 
 distinguish them. I don't have strong opinions about our current mFoo naming 
 versus the google-like member_ naming.

I have a tool for that: static analysis tools usually have a shadowing 
detection function.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: List of deprecated constructs [was Re: A proposal to reduce the number of styles in Mozilla code]

2014-01-07 Thread Joshua Cranmer 

On 1/7/2014 12:58 PM, Benoit Jacob wrote:
I would like a random voice in favor of deprecating NS_ENSURE_* for 
the reason that hiding control flow behind macros is IMO one of the 
most evil usage patterns of macros.


In general, I would agree. I have no problems with killing, say, 
NS_ENSURE_TRUE or similar macros. However, NS_ENSURE_SUCCESS is a 
special case: it is the equivalent in JS of not providing a try/catch 
statement around a JS expression. In code that makes extremely heavy use 
of XPCOM requirements, NS_ENSURE_SUCCESS brings the amount of 
boilerplate needed to one line per function; to not use it requires 
significantly more boilerplate (three lines, if you insist on following 
the always-brace-ifs convention).


--
Joshua Cranmer
Thunderbird and DXR developer
Source code archæologist

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS

2014-01-07 Thread Benjamin Smedberg

On 1/6/2014 7:43 PM, smaug wrote:



Why this deprecation?


Karl is right, we are removing the macros that hide control flow (as 
well as warnings, in this case).


I'm considering the NS_WARN_IF_FAILED(rv) proposal. It's obviously a 
less typing then NS_WARN_IF(NS_FAILED(rv)), but I'm not convinced that 
it's clear just by reading it what the return type would be, especially 
since the signature of NS_WARN_IF returns the same value that is passed in:


bool NS_WARN_IF(bool);

I'm trying to figure out if people would expect the result of 
NS_WARN_IF_FAILED to be an nsresult or a bool.


--BDS

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Adam Roach

On 1/7/14 12:16, Martin Thomson wrote:

On 2014-01-06, at 19:28, Patrick McManus mcma...@ducksong.com wrote:


I strongly prefer at least a 100 character per line limit. Technology
marches on.

Yes.  I’ve encountered too many instances where 80 was not enough.


Since people are introducing actual research information here, let's run 
some numbers. According to Paterson et. al. [1], reading comprehension 
speed is actively hindered by lines that are either too short or too 
long, which they define as 9 picas (1.5 inches) and 43 picas (~7 
inches), respectively. Comprehension is significantly faster at 19 picas 
(~3 inches).


Using the default themes that ship with the OS X Terminal app, an 
80-character-wide terminal is on the order of 4 inches wide on a 15-inch 
monitor. 100 columns pushes this to nearly 5 inches.


Now, I'm not arguing for a 60-character line length here. However, it 
would seem that moving from 80 to 100 is going in the wrong direction 
for comprehension speed.



[1] http://psycnet.apa.org/journals/xge/27/5/572/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Boris Zbarsky

On 1/7/14 2:29 PM, Mike Hoye wrote:

- Indent with 4 spaces,


Mike, do you have the background on why 4 is preferred to 2?  The things 
you cite don't seem to differentiate between these two options...


One reason I've seen 2 preferred to 4 (apart from keeping line lengths 
down) is that:


  if (somethingHere() 
  somethingElse())
  doSomething();

is less clear about what's condition and what's body the if body is than:

  if (somethingHere() 
  somethingElse())
doSomething();

but this would obviously also be affected by the bracing policy.

-Boris
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Adam Roach

On 1/7/14 14:23, Boris Zbarsky wrote:
One reason I've seen 2 preferred to 4 (apart from keeping line lengths 
down)...


Thanks. I was just about to raise the issue that choosing four over two 
has no identified benefits, and only serves to exacerbate *both* sides 
of the argument over line length limits.


--
Adam Roach
Principal Platform Engineer
a...@mozilla.com
+1 650 903 0800 x863
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: A proposal to reduce the number of styles in Mozilla code

2014-01-07 Thread Benjamin Smedberg

On 1/5/2014 9:34 PM, Nicholas Nethercote wrote:




   I propose that we officially remove this implicit discouragement, and even
   encourage changes that convert non-Mozilla-style code to Mozilla-style (with
   some exceptions; see below). When modifying badly-styled code, following
   existing style is still probably best.
I don't think that style changes are currently actively forbidden, but 
they aren't exactly encouraged. What you're primarily changing is 
whether they are encouraged.


I think that there are two important rules here:

* Real patches shouldn't mess with coding style
* Style changes should be coordinated with the module owner to minimize 
conflicts with other patches that are in progress or planned.


Karl suggests in the thread that we should discourage reformatting which 
moves code to a different line, which would appear to include braces. I 
disagree, since bracing changes and whitespace formatting are the most 
important issues for scanning code, and we should fix them all at once.


What's not clear to me from the discussion is whether there is a 
reformatting tool which already exists which can do what we need. If 
there is, I think we should actively discourage people reformatting by 
hand, and just encourage people to use the tool at an appropriate time.





- There is an semi-official policy that the owner of a module can dictate its
   style. Examples: SpiderMonkey, Storage, MFBT.
Spidermonkey is hard because it uses a different member naming 
convention and so the changes are potentially much more invasive. But I 
at least support starting out by unifying the indentation and bracing 
styles across the tree, and removing other exceptions.


   Also, we probably shouldn't change the style of imported third-party code;

Yes.

I am the owner of at least the C/C++ portions of the style guide; I 
propose to wait and see whether the C++ reformatting tools are of 
sufficient quality that we can use them directly, to avoid 
hand-reformatting, and make a final decision next week sometime.


In the meantime, we should wrap up the pending discussions about other 
changes to the style guide, such as 80/100/infinite columns, 
member/parameter/local naming convention, and other threads that Gavin 
was going to start and one I'm going to start now ;-)


--BDS

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Style guide clarity on C++-isms

2014-01-07 Thread Benjamin Smedberg
There are a few C++-isms which vary widely across the tree and I'd like 
to clarify before we start any major refactorings.


1) Bracing of method bodies in a C++ class declaration

Currently, C++ method bodies inline within a class declaration are 
documented to start on the next line, e.g.


class B
{
public:
  void Method()
  {
 // Inline body brace is on the next line, column 2
  }
};

Mozilla code widely puts the opening bracde of inline bodies such as 
this on the end of the declaration line, and I want to make sure we're 
in agreement to fix this.


2) indentation of visibility within a class declaration

In the above example, public is at the same indentation level as the 
class. This is common in new code, but I want to call it out.


3) placement of the colon and commas for C++ member initializers

MyClass::MyClass()
  : mMember1(foo)
  , mMember2(foo)
{
  // function body
}

Existing usage is all over the place. I personally have found this style 
to produce the least number of merge conflicts when applying patches to 
these kinds of methods, but again I want to make sure we're in agreement.


--BDS

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Mike Hoye

On 1/7/2014, 3:22 PM, Adam Roach wrote:


Since people are introducing actual research information here, let's 
run some numbers. According to Paterson et. al. [1], reading 
comprehension speed is actively hindered by lines that are either too 
short or too long, which they define as 9 picas (1.5 inches) and 43 
picas (~7 inches), respectively. Comprehension is significantly faster 
at 19 picas (~3 inches). 
[...]

[1] http://psycnet.apa.org/journals/xge/27/5/572/


I found some other citations about this as well - 
http://www.humanfactors.com/downloads/feb03.asp#kath  among others - but 
they all measure people reading English, not code. I haven't been able 
to find any that are specifically on-point for coding, but I'm still 
looking.


2 v. 4 spaces: more on that shortly.


- mhoye
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Style guide clarity on C++-isms

2014-01-07 Thread Robert O'Callahan
I agree that those are the current best practices.

We have a lot of places where we write void Method() { ... } all on one
line for trivial setters and getters. I wonder if we should permit that.

We have a lot of places where the opening brace of a class declaration is
on the same line as the class name. We should stop doing that. Similar for
function declarations.

Rob
-- 
Jtehsauts  tshaei dS,o n Wohfy  Mdaon  yhoaus  eanuttehrotraiitny  eovni
le atrhtohu gthot sf oirng iyvoeu rs ihnesa.rt sS?o  Whhei csha iids  teoa
stiheer :p atroa lsyazye,d  'mYaonu,r  sGients  uapr,e  tfaokreg iyvoeunr,
'm aotr  atnod  sgaoy ,h o'mGee.t  uTph eann dt hwea lmka'n?  gBoutt  uIp
waanndt  wyeonut  thoo mken.o w
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Robert O'Callahan
On Wed, Jan 8, 2014 at 9:46 AM, Mike Hoye mh...@mozilla.com wrote:

 On 1/7/2014, 3:22 PM, Adam Roach wrote:


 Since people are introducing actual research information here, let's run
 some numbers. According to Paterson et. al. [1], reading comprehension
 speed is actively hindered by lines that are either too short or too long,
 which they define as 9 picas (1.5 inches) and 43 picas (~7 inches),
 respectively. Comprehension is significantly faster at 19 picas (~3
 inches). [...]
 [1] http://psycnet.apa.org/journals/xge/27/5/572/


 I found some other citations about this as well -
 http://www.humanfactors.com/downloads/feb03.asp#kath  among others - but
 they all measure people reading English, not code.


Yes, it's not clear that those English results would carry over. For one
thing, when reading English text almost every line is going to be close to
the maximum line length, but not so in code.

Rob
-- 
Jtehsauts  tshaei dS,o n Wohfy  Mdaon  yhoaus  eanuttehrotraiitny  eovni
le atrhtohu gthot sf oirng iyvoeu rs ihnesa.rt sS?o  Whhei csha iids  teoa
stiheer :p atroa lsyazye,d  'mYaonu,r  sGients  uapr,e  tfaokreg iyvoeunr,
'm aotr  atnod  sgaoy ,h o'mGee.t  uTph eann dt hwea lmka'n?  gBoutt  uIp
waanndt  wyeonut  thoo mken.o w
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Fitzgerald, Nick
On 1/7/14 11:23 AM, Neil wrote:
 Martin Thomson wrote:

 in JS, a case that I’ve encountered several times:
   _classMethodName: function
 ReasonableClassName__classMethodName(argument1, argument2) {

 I thought that our debugging story had improved sufficiently that we
 didn't need these fake function names any more.

Yup. You shouldn't need to use named function expressions to get useful
stack traces anymore.

At least in devtools, new files don't use them and there are bugs open
for removing them in existing files.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


MemShrink Meeting - Tuesday, 7 January 2014 at 2:00 PM PST

2014-01-07 Thread Kyle Huey
Sorry for the short notice.

The wiki page for this meeting is at:
   https://wiki.mozilla.org/Performance/MemShrink

Agenda:
* Prioritize unprioritized MemShrink bugs.
* Discuss how we measure progress.
* Discuss approaches to getting more data.

Meeting details:

* Tue, 7 January, 2:00 PM PST
*
http://arewemeetingyet.com/Los%20Angeles/2014-1-7/14:00/MemShrink%20Meeting
* Vidyo: Memshrink
* SF: Noise Pop. 7th Floor
* Dial-in Info:
   - In office or soft phone: extension 92
   - US/INTL: 650-903-0800 or 650-215-1282 then extension 92
   - Toll-free: 800-707-2533 then password 369
   - Conference num 98802
__
_
dev-planning mailing list
dev-plann...@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-planning
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Ehsan Akhgari

On 1/7/2014, 2:29 PM, Martin Thomson wrote:

On 2014-01-07, at 11:28, Benjamin Smedberg benja...@smedbergs.us wrote:


Especially for new contributors we shouldn't assume that most editors can do 
this correctly. I personally think that the risk of introducing a new member 
and then having locals shadow members is real enough that we should somehow 
distinguish them. I don't have strong opinions about our current mFoo naming 
versus the google-like member_ naming.


I have a tool for that: static analysis tools usually have a shadowing 
detection function.


But many people don't have those tools, so this is not a good argument. 
 (Note that we don't even turn on -Wshadow currently)


Cheers,
Ehsan

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Valgrind testing is now visible on TBPL

2014-01-07 Thread Nicholas Nethercote
I am happy to announce that the Valgrind test job is now visible on TBPL. The
abbreviation for the job is V, and it only runs on Linux64 opt
builds.  Any failures
are cause for a patch to be backed out.

The job runs on try, and can be run locally (definitely on Linux, possibly on
Mac, not on Windows) with |mach valgrind-test|.  Full documentation is at
https://developer.mozilla.org/en-US/docs/Valgrind_test_job.

Please let me know if any problems crop up.

The coverage is currently quite poor. The Valgrind test job runs the
same workload as the PGO profiling run (see
http://mxr.mozilla.org/mozilla-central/source/build/pgo/index.html?force=1),
which just loads four simple static pages and also SunSpider. Given that the
job takes ~45 minutes and ~38 minutes of that is build time and only ~7 minutes
is Valgrind time, there's scope for increasing coverage significantly without
making the job much slower. I'd be interested to hear any suggestions for
inputs that should be tested. (Unfortunately, Valgrind is slow enough that
things like full test suites aren't feasible, and so hand-crafting test inputs
makes the most sense.)

Nick
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Jim Porter

On 01/06/2014 08:23 PM, Karl Tomlinson wrote:

Yes, those are the sensible options.

Wrapping at  80 columns just makes things worse for those that
like to save some screen room for something else, view code on a
mobile device, etc.


I for one prefer wrapping at 80 columns because with my font settings, I 
can have 3 buffers open side-by-side. I generally find that a lot more 
useful than the vertical space that would be saved by wrapping at, say, 
100 columns.


- Jim

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Patrick McManus
Typically I have to choose between
 1] 80 columns
 2] descriptive and non-abbreviated naming
 3] displaying a logic block without scrolling

to me, #1 is the least valuable.



On Tue, Jan 7, 2014 at 4:51 PM, Jim Porter jpor...@mozilla.com wrote:

 On 01/06/2014 08:23 PM, Karl Tomlinson wrote:

 Yes, those are the sensible options.

 Wrapping at  80 columns just makes things worse for those that
 like to save some screen room for something else, view code on a
 mobile device, etc.


 I for one prefer wrapping at 80 columns because with my font settings, I
 can have 3 buffers open side-by-side. I generally find that a lot more
 useful than the vertical space that would be saved by wrapping at, say, 100
 columns.

 - Jim


 ___
 dev-platform mailing list
 dev-platform@lists.mozilla.org
 https://lists.mozilla.org/listinfo/dev-platform

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Nicholas Nethercote
On Tue, Jan 7, 2014 at 11:28 AM, Benjamin Smedberg
benja...@smedbergs.us wrote:
 I don't have strong opinions about our current mFoo naming
 versus the google-like member_ naming.

If you're just distinguishing members, then |foo_| is good. But if
you're distinguishing parameters and globals/statics as well (which I
think is a good idea), then mFoo/aFoo/gFoo/sFoo makes more sense.

Nick
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: A proposal to reduce the number of styles in Mozilla code

2014-01-07 Thread Ehsan Akhgari

On 1/7/2014, 4:55 PM, Chris Peterson wrote:

On 1/7/14, 12:31 PM, Benjamin Smedberg wrote:

What's not clear to me from the discussion is whether there is a
reformatting tool which already exists which can do what we need. If
there is, I think we should actively discourage people reformatting by
hand, and just encourage people to use the tool at an appropriate time.


The astyle and uncrustify reformatting tools have options to add
braces.


http://astyle.sourceforge.net/
http://uncrustify.sourceforge.net/


I'm also planning to see what it would take to teach clang-format about 
braces...


Cheers,
Ehsan

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Ehsan Akhgari

On 1/7/2014, 5:49 PM, Nicholas Nethercote wrote:

On Tue, Jan 7, 2014 at 11:28 AM, Benjamin Smedberg
benja...@smedbergs.us wrote:

I don't have strong opinions about our current mFoo naming
versus the google-like member_ naming.


If you're just distinguishing members, then |foo_| is good. But if
you're distinguishing parameters and globals/statics as well (which I
think is a good idea), then mFoo/aFoo/gFoo/sFoo makes more sense.


And because I think that distinguishing parameters is at least very 
useful, I think we should maintain mFoo and aFoo.  And sFoo for static 
members.  I don't have any opinions on gFoo.


Cheers,
Ehsan
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Style guide clarity on C++-isms

2014-01-07 Thread Ehsan Akhgari

On 1/7/2014, 3:53 PM, Robert O'Callahan wrote:

I agree that those are the current best practices.

We have a lot of places where we write void Method() { ... } all on one
line for trivial setters and getters. I wonder if we should permit that.


I'd rather if we didn't.  Often times changing the name/type of 
something will make that go past the 80/100/whatever line length limit 
and you'd have to indent it anyways.  Plus, consistency FTW.



We have a lot of places where the opening brace of a class declaration is
on the same line as the class name. We should stop doing that. Similar for
function declarations.


Agreed on both.

Cheers,
Ehsan

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Martin Thomson

On 2014-01-07, at 14:52, Ehsan Akhgari ehsan.akhg...@gmail.com wrote:

 I don't have any opinions on gFoo.

Aside from “should not exist” ?
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Style guide clarity on C++-isms

2014-01-07 Thread Cameron McCormack

Benjamin Smedberg wrote:

1) Bracing of method bodies in a C++ class declaration

Currently, C++ method bodies inline within a class declaration are
documented to start on the next line, e.g.

class B
{
public:
void Method()
{
// Inline body brace is on the next line, column 2
}
};

Mozilla code widely puts the opening bracde of inline bodies such as
this on the end of the declaration line, and I want to make sure we're
in agreement to fix this.


I don't mind too much.

If we do make it same-line, it would be nice to write down how to handle 
empty inline method/constructor bodies, too:


  class A
  {
A(int aMember)
  : mMember(aMember) {
}
  };

or

  class A
  {
A(int aMember)
  : mMember(aMember) { }
  };

or

  class A
  {
A(int aMember)
  : mMember(aMember) {}
  };

Also, I have seen (and written) short one-line method bodies like this:

  class A
  {
bool IsFlagSet() const
  { return mFlag; }
  };

which I guess should be invalid with brace-on-first-line.  I think I 
actually prefer (and don't mind the churn that Ehsan mentions if we 
rename things)


  class A
  {
bool IsFlagSet() const { return mFlag; }
  };

if it's short enough, and if not, then we go to:

  class A
  {
bool IsFlagSet() const {
  return mFlag;
}
  };


2) indentation of visibility within a class declaration

In the above example, public is at the same indentation level as the
class. This is common in new code, but I want to call it out.

3) placement of the colon and commas for C++ member initializers

MyClass::MyClass()
: mMember1(foo)
, mMember2(foo)
{
// function body
}

Existing usage is all over the place. I personally have found this style
to produce the least number of merge conflicts when applying patches to
these kinds of methods, but again I want to make sure we're in agreement.


Agree.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Cameron McCormack

Patrick McManus wrote:

Typically I have to choose between
  1] 80 columns
  2] descriptive and non-abbreviated naming
  3] displaying a logic block without scrolling

to me, #1 is the least valuable.


Thoroughly agree.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Cameron McCormack
Speaking of the troubles with 80 character line lengths, I find I often 
need to write variable and function declaration/calls like this:


https://hg.mozilla.org/mozilla-central/file/8f1c9cdedba5/layout/style/nsCSSParser.cpp#l429
https://hg.mozilla.org/mozilla-central/file/8f1c9cdedba5/layout/style/nsCSSParser.cpp#l2182

I think this kind of wrapping, where if you can't fit the first argument 
on the line then you put it on a new line, but leave the opening paren 
on the first line, is reasonably common.  I tend to align the arguments 
so that they right align with the 80 character column (or somewhere 
close), although that then can result in unnecessary churn when 
renaming/adding/removing arguments.  It would be good if the style guide 
could say what to do here.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Style guide clarity on C++-isms

2014-01-07 Thread Ehsan Akhgari

On 1/7/2014, 6:18 PM, Cameron McCormack wrote:

Benjamin Smedberg wrote:

1) Bracing of method bodies in a C++ class declaration

Currently, C++ method bodies inline within a class declaration are
documented to start on the next line, e.g.

class B
{
public:
void Method()
{
// Inline body brace is on the next line, column 2
}
};

Mozilla code widely puts the opening bracde of inline bodies such as
this on the end of the declaration line, and I want to make sure we're
in agreement to fix this.


I don't mind too much.

If we do make it same-line, it would be nice to write down how to handle
empty inline method/constructor bodies, too:

   class A
   {
 A(int aMember)
   : mMember(aMember) {
 }
   };

or

   class A
   {
 A(int aMember)
   : mMember(aMember) { }
   };

or

   class A
   {
 A(int aMember)
   : mMember(aMember) {}
   };


Exactly.  If we require braces on their own lines for function bodies 
everywhere, we wouldn't need to solve this!



Also, I have seen (and written) short one-line method bodies like this:

   class A
   {
 bool IsFlagSet() const
   { return mFlag; }
   };

which I guess should be invalid with brace-on-first-line.  I think I
actually prefer (and don't mind the churn that Ehsan mentions if we
rename things)

   class A
   {
 bool IsFlagSet() const { return mFlag; }
   };

if it's short enough, and if not, then we go to:

   class A
   {
 bool IsFlagSet() const {
   return mFlag;
 }
   };


See my reply to this here.  I think when in doubt here, consistency 
should win.  So, yay to brace on its own line for all function bodies.


Cheers,
Ehsan
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Style guide clarity on C++-isms

2014-01-07 Thread Cameron McCormack

Ehsan Akhgari wrote:

Exactly. If we require braces on their own lines for function bodies
everywhere, we wouldn't need to solve this!


Are you sure? :)  There are a bunch of instances of

  class A
  {
A(int aMember)
  : mMember(aMamber)
{}
  };

through the tree.  Depends how the braces on new line rule is written, 
of course.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: A proposal to reduce the number of styles in Mozilla code

2014-01-07 Thread Nicholas Nethercote
On Tue, Jan 7, 2014 at 12:31 PM, Benjamin Smedberg
benja...@smedbergs.us wrote:

 I am the owner of at least the C/C++ portions of the style guide; I propose
 to wait and see whether the C++ reformatting tools are of sufficient quality
 that we can use them directly, to avoid hand-reformatting, and make a final
 decision next week sometime.

Thanks! Who will evaluate these tools?

BTW, jcranmer's point above about there being three categories of
rules (formatting, naming, other) is a good one. Tools can only help
with the first category (and possibly not even all rules within that
category). So some hand-reformatting will be inevitable.

Nick
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Reftests execute differently on Android or b2g?

2014-01-07 Thread Neil
I tried to check in a reftest today. Apparently it fails on Android and 
b2g. The failure mode appears to be that the reftest takes a screenshot 
before the test has loaded (the page is still blank, whereas it should 
have a red square for failure or a green square for success). Do 
reftests execute differently on those platforms?


--
Warning: May contain traces of nuts.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: List of deprecated constructs [was Re: A proposal to reduce the number of styles in Mozilla code]

2014-01-07 Thread Neil

Benoit Jacob wrote:


I'm scanning a function for possible early returns


Good thing you don't have to deal with C++ exceptions then.

--
Warning: May contain traces of nuts.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: A proposal to reduce the number of styles in Mozilla code

2014-01-07 Thread Mike Hommey
On Tue, Jan 07, 2014 at 04:13:20PM -0800, Nicholas Nethercote wrote:
 On Tue, Jan 7, 2014 at 12:31 PM, Benjamin Smedberg
 benja...@smedbergs.us wrote:
 
  I am the owner of at least the C/C++ portions of the style guide; I propose
  to wait and see whether the C++ reformatting tools are of sufficient quality
  that we can use them directly, to avoid hand-reformatting, and make a final
  decision next week sometime.
 
 Thanks! Who will evaluate these tools?
 
 BTW, jcranmer's point above about there being three categories of
 rules (formatting, naming, other) is a good one. Tools can only help
 with the first category (and possibly not even all rules within that
 category). So some hand-reformatting will be inevitable.

There are actually clang-based tools for e.g. variables renaming.

Mike
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Jeff Gilbert
FWIW, WebGL and GLContext do this for the same reasons.

-jgilbert

- Original Message -
From: Jeff Walden jwalden+...@mit.edu
To: dev-platform@lists.mozilla.org
Sent: Tuesday, January 7, 2014 2:26:46 PM
Subject: Re: Mozilla style guide issues, from a JS point of view

On 01/07/2014 02:23 PM, Boris Zbarsky wrote:
 One reason I've seen 2 preferred to 4 (apart from keeping line lengths down) 
 is that:
 
   if (somethingHere() 
   somethingElse())
   doSomething();
 
 is less clear about what's condition and what's body the if body is than:
 
   if (somethingHere() 
   somethingElse())
 doSomething();
 
 but this would obviously also be affected by the bracing policy.

Regarding the effect of the bracing policy, SpiderMonkey used to have

if (somethingHere() 
somethingElse()) {
doSomething();
}

which was unreadable.  You simply can't easily skim and see where the body 
starts and where the condition ends, even with braces.  We shoved the opening 
brace to its own line:

if (somethingHere() 
somethingElse())
{
doSomething();
}

It doesn't seem to me that the four-space issue here is helped by braces in 
end-of-line configuration.

Jeff
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: A proposal to reduce the number of styles in Mozilla code

2014-01-07 Thread Nicholas Nethercote
On Tue, Jan 7, 2014 at 4:38 PM, Mike Hommey m...@glandium.org wrote:
 So some hand-reformatting will be inevitable.

 There are actually clang-based tools for e.g. variables renaming.

Thanks for the nit. I believe the broader point -- that tools won't
fix every style issue, so some hand-reformatting will be necessary --
still stands.

Nick
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Mark Finkle
 One reason I've seen 2 preferred to 4 (apart from keeping line lengths
 down) is that:

 if (somethingHere() 
 somethingElse())
 doSomething();

 is less clear about what's condition and what's body the if body is than:

 if (somethingHere() 
 somethingElse())
 doSomething();

Changing the line length policy would also avoid needing to wrap the multiple 
conditions onto separate lines. 
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Mark Finkle
Agreed. In /mobile JS code we favor wrapping 80 characters. 100 seems 
reasonable to me. 

- Original Message -

 I strongly prefer at least a 100 character per line limit. Technology
 marches on.

 On Mon, Jan 6, 2014 at 9:23 PM, Karl Tomlinson mozn...@karlt.net wrote:

  L. David Baron writes:
 
   I tend to think that we should either:
   * stick to 80
   * require no wrapping, meaning that comments must be one paragraph
   per line, boolean conditions must all be single line, and assume
   that people will deal, using an editor that handles such code
   usefully
 
  Yes, those are the sensible options.
 
  Wrapping at  80 columns just makes things worse for those that
  like to save some screen room for something else, view code on a
  mobile device, etc.
  ___
  dev-platform mailing list
  dev-platform@lists.mozilla.org
  https://lists.mozilla.org/listinfo/dev-platform
 
 ___
 dev-platform mailing list
 dev-platform@lists.mozilla.org
 https://lists.mozilla.org/listinfo/dev-platform
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: List of deprecated constructs [was Re: A proposal to reduce the number of styles in Mozilla code]

2014-01-07 Thread Kyle Huey
On Tue, Jan 7, 2014 at 11:29 AM, Benoit Jacob jacob.benoi...@gmail.comwrote:

 For example, if I'm scanning a function for possible early returns (say I'm
 debugging a bug where we're forgetting to close or delete a thing before
 returning), I now need to scan for NS_ENSURE_SUCCESS in addition to
 scanning for return. That's why hiding control flow in macros is, in my
 opinion, never acceptable.


If you care about that 9 times out of 10 you are failing to use an RAII
class when you should be.

Since we seem to be voting now, I am moderately opposed to making XPCOM
method calls more boilerplate-y, and very opposed to removing
NS_ENSURE_SUCCESS without some sort of easy shorthand to test an nsresult
and print to the console if it is a failure.  I know for sure that some of
the other DOM peers (smaug and bz come to mind) feel similarly about the
latter.

- Kyle
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: JavaScript Style Guide. Emacs mode line.

2014-01-07 Thread ISHIKAWA,chiaki
(2013/09/10 19:17), ishikawa wrote:

  [ omissions ]
 
 I am getting the hang of emacs mode line.
 
 /* -*- Mode: javascript; tab-width: 8; indent-tabs-mode: nil;
 c-basic-offset: 2 ; js-indent-level : 2 ; js-curly-indent-offset: 0 -*- */
 /* vim: set ts=2 et sw=2 tw=80: */
 
 seem to do the job.
 
 I have not tested vim part.
 We can probably omit js-curly-inent-offset here, but
 for now, I am keeping it as a reminder.
 It seems that js mode in Emacs worked with c-basic-offset a couple of years
 ago, but today's js-mode (Javascript mode is an alias)
 needs its own variables, I think.
 
 

I have been tinkering with javascript mode line for Emacs.

/* -*- Mode: javascript; tab-width: 8; indent-tabs-mode: nil;
c-basic-offset: 2 ; js-indent-level : 2  -*- */

With the current Emacs and its preferred javascript mode, the above mode
line seems to work rather well.
js-indent-level setting (set to 2) seems to be the key
for proper indentation.

But there do exist situations where the above mode line is not dictating
specific requirements enough.
(Specifically the placing of period in a long series of
 a.b.c.d.e, and the IDs are actually lengthy and so
 lines are broken as in
 t = a.bb
  .c.d.e
(Or was it
 t = a.bb.
   c.d.e ?)


Does anyone have something better than the above simple mode line?

TIA



___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: List of deprecated constructs [was Re: A proposal to reduce the number of styles in Mozilla code]

2014-01-07 Thread Mike Habicher

On 14-01-07 08:04 PM, Kyle Huey wrote:

On Tue, Jan 7, 2014 at 11:29 AM, Benoit Jacob jacob.benoi...@gmail.comwrote:


For example, if I'm scanning a function for possible early returns (say I'm
debugging a bug where we're forgetting to close or delete a thing before
returning), I now need to scan for NS_ENSURE_SUCCESS in addition to
scanning for return. That's why hiding control flow in macros is, in my
opinion, never acceptable.


If you care about that 9 times out of 10 you are failing to use an RAII
class when you should be.

Since we seem to be voting now, I am moderately opposed to making XPCOM
method calls more boilerplate-y, and very opposed to removing
NS_ENSURE_SUCCESS without some sort of easy shorthand to test an nsresult
and print to the console if it is a failure.  I know for sure that some of
the other DOM peers (smaug and bz come to mind) feel similarly about the
latter.


Would a macro starting with RETURN_ be an improvement over NS_ENSURE_?

e.g.
  nsresult rv = Foo();
  RETURN_AND_WARN_IF_FAIL(rv);

It's a mouthful (handful?) to type, but it's a single line and makes 
explicit what's going on.


--m.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: JavaScript Style Guide. Emacs mode line.

2014-01-07 Thread Mike Hommey
On Wed, Jan 08, 2014 at 10:23:28AM +0900, ISHIKAWA,chiaki wrote:
 (2013/09/10 19:17), ishikawa wrote:
 
   [ omissions ]
  
  I am getting the hang of emacs mode line.
  
  /* -*- Mode: javascript; tab-width: 8; indent-tabs-mode: nil;
  c-basic-offset: 2 ; js-indent-level : 2 ; js-curly-indent-offset: 0 -*- */
  /* vim: set ts=2 et sw=2 tw=80: */
  
  seem to do the job.
  
  I have not tested vim part.
  We can probably omit js-curly-inent-offset here, but
  for now, I am keeping it as a reminder.
  It seems that js mode in Emacs worked with c-basic-offset a couple of years
  ago, but today's js-mode (Javascript mode is an alias)
  needs its own variables, I think.
  
  
 
 I have been tinkering with javascript mode line for Emacs.
 
 /* -*- Mode: javascript; tab-width: 8; indent-tabs-mode: nil;
 c-basic-offset: 2 ; js-indent-level : 2  -*- */

Do we need so much boilerplate in all our files? With this and
vim-modeline, we now have editor boilerplate that takes as much space as
the MPL boilerplate. Also, afaik, vim modelines are mostly useless, since
vim doesn't use them by default anyways (at least, it doesn't on
Debian). And emacs can take local variables in a .dir-locals.el file.
Why not put one at the root directory in mozilla-central? (and others if
necessary in subdirectories for overrides)

Mike
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: JavaScript Style Guide. Emacs mode line.

2014-01-07 Thread Nicholas Nethercote
On Tue, Jan 7, 2014 at 5:38 PM, Mike Hommey m...@glandium.org wrote:
 Also, afaik, vim modelines are mostly useless, since
 vim doesn't use them by default anyways

I enable them, and as long as we have some files with 2-space indents
and some with 4-space indents I will continue to find them useful...

Nick
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: JavaScript Style Guide. Emacs mode line.

2014-01-07 Thread Martin Thomson
On 2014-01-07, at 17:38, Mike Hommey m...@glandium.org wrote:

 Do we need so much boilerplate in all our files? With this and
 vim-modeline, we now have editor boilerplate that takes as much space as
 the MPL boilerplate. Also, afaik, vim modelines are mostly useless, since
 vim doesn't use them by default anyways (at least, it doesn't on
 Debian). And emacs can take local variables in a .dir-locals.el file.
 Why not put one at the root directory in mozilla-central? (and others if
 necessary in subdirectories for overrides)

Yeah, I thought that was the whole point of having a .emacs file.  I’ve never 
found those mode line things to be properly useful short of the point that it 
includes the entire c-offsets-alist for the buffer.  I’d rather the mode line 
stuff be removed.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Boris Zbarsky

On 1/7/14 7:57 PM, Mark Finkle wrote:

Changing the line length policy would also avoid needing to wrap the multiple 
conditions onto separate lines.


I often wrap conditions just to make the more readable... Something like 
this:


  if ((x || y)  (z || w))

is a lot less readable for me than:

  if ((x || y) 
  (z || w))

once the actual expressions around || get a bit longer, even if the 
former isn't hitting a line length limit.


-Boris

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Mozilla style guide issues, from a JS point of view

2014-01-07 Thread Joshua Cranmer
On 1/6/2014 7:38 PM, L. David Baron wrote:
 I tend to think that we should either:
  * stick to 80
  * require no wrapping, meaning that comments must be one paragraph
per line, boolean conditions must all be single line, and assume
that people will deal, using an editor that handles such code
usefully

Since I'm seeing a lot of people advocating that the wrap margin should be 100,
let me reiterate
David Baron's comment that the wrap margin must either be 80 or infinite with a
self-demonstrating
response. I once suggested a wider wrap margin as a compromise years ago, but I
have come around to
the viewpoint that the only acceptable wrap margins are 80 and infinite.

The absolute worst visual display to read is when you have text that contains a
mixture of soft
wraps (caused by lines longer than your display) with hard wraps (caused by a
max-line-length
requirement). The text in this post is set up to emulate the display of text
that has a hard wrap at
around the 100th character being displayed on an 80-character terminal window.
Thus, this is the
code that people using 80-character terminal windows will be subjected to in
places that have heavy
paragraph-style text (e.g., README files, documentation comments, or even really
long regular
comment blocks).

Now, you can argue that people should just resize their windows to be insert
desired wrap margin
here. I suspect that many of the people making this arguments are people who
use GUI editors that
have 120+-character viewports for code, arguing for the ability to utilize what
it is often just
dead whitespace in their editor and frustrated that people using older
technology are limiting use
of this space. As one who uses 80-character terminals heavily, I can report that
changing the size
of the terminal is generally not a viable option. Many people who use smaller
terminal sizes fill up
the screen real estate by tiling the terminals. On smaller resolutions and
larger font sizes,
changing the screen size even to 100 makes it impossible to tile more than two
windows horizontally
on the screen: changing the screen size is impossible. Furthermore, the default
size for a terminal
has been decided on by universal convention to be 80x24: any would-be
contributor to Mozilla who
uses the default-sized terminals would be forced to either put up with the
painful reading of
poorly-wrapped lines or to figure out how to retool their entire workflow just
to be able to
contribute--and I suspect that many would instead find themselves driven away.

Infinitely-long lines do not have the same problems that wrapping at any value
 80 does: most
editors are capable of soft-wrapping them to some degree of legibility. The
problem isn't that some
lines have to be wrapped as much as it is that the lines are wrapped at
completely wrong places.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: JavaScript Style Guide. Emacs mode line.

2014-01-07 Thread Chris Peterson

On 1/7/14, 5:42 PM, Martin Thomson wrote:

Yeah, I thought that was the whole point of having a .emacs file.  I’ve never 
found those mode line things to be properly useful short of the point that it 
includes the entire c-offsets-alist for the buffer.  I’d rather the mode line 
stuff be removed.


If mozilla-central is reformatted, that would be a good time to also 
remove the modelines.


A possible timeline of events:

1. Finish bikeshedding coding style
2. Update official style guide (owner=bsmedberg?)
3. Add style config files for vim/emacs/clang-format in mozilla-central
4. Reformat mozilla-central code (piecemeal or big bang)
5. Remove modelines from mozilla-central files
6. Add lint tools for patch style (bug 875605 and others)


chris
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: List of deprecated constructs [was Re: A proposal to reduce the number of styles in Mozilla code]

2014-01-07 Thread Benoit Jacob
2014/1/7 Kyle Huey m...@kylehuey.com

 On Tue, Jan 7, 2014 at 11:29 AM, Benoit Jacob jacob.benoi...@gmail.comwrote:

 For example, if I'm scanning a function for possible early returns (say
 I'm
 debugging a bug where we're forgetting to close or delete a thing before
 returning), I now need to scan for NS_ENSURE_SUCCESS in addition to
 scanning for return. That's why hiding control flow in macros is, in my
 opinion, never acceptable.


 If you care about that 9 times out of 10 you are failing to use an RAII
 class when you should be.


I was talking about reading code, not writing code. I spend more time
reading code that I didn't write, than writing code. Of course I do use
RAII helpers when I write this kind of code myself, in fact just today I
landed two more such helpers in gfx/gl/ScopedGLHelpers.* ...

Benoit



 Since we seem to be voting now, I am moderately opposed to making XPCOM
 method calls more boilerplate-y, and very opposed to removing
 NS_ENSURE_SUCCESS without some sort of easy shorthand to test an nsresult
 and print to the console if it is a failure.  I know for sure that some of
 the other DOM peers (smaug and bz come to mind) feel similarly about the
 latter.

 - Kyle

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: JavaScript Style Guide. Emacs mode line.

2014-01-07 Thread Karl Tomlinson
Chris Peterson writes:

 If mozilla-central is reformatted, that would be a good time to
 also remove the modelines.

 5. Remove modelines from mozilla-central files

I'm happy with removal of modelines provided .dir-locals.el files
are added to provide the same functionality.

There is imported code in mozilla-central that will not be
reformatted.

Also, removal the functionality would add another deterrent to new
contributors, or others, who like edit code outside
mozilla-central.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: JavaScript Style Guide. Emacs mode line.

2014-01-07 Thread Blair McBride
I'd like to see the removal of the modelines also. A root config file is 
much cleaner.


For the widest possible support of editors, I'd love to see a root 
.editorconfig file. See http://editorconfig.org/ - it's an 
editor-neutral config, with plugins for many editors/IDEs (including 
Emacs, Vim, SublimeText, etc). And it supports different settings for 
different filetypes, so we can have different settings for C++, JS, CSS, 
etc.


- Blair




On 8/01/2014 5:33 p.m., Karl Tomlinson wrote:

Chris Peterson writes:


If mozilla-central is reformatted, that would be a good time to
also remove the modelines.



5. Remove modelines from mozilla-central files


I'm happy with removal of modelines provided .dir-locals.el files
are added to provide the same functionality.

There is imported code in mozilla-central that will not be
reformatted.

Also, removal the functionality would add another deterrent to new
contributors, or others, who like edit code outside
mozilla-central.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform