Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Delta Between Two Patch Sets: pages/coding-style.html

Issue 29371951: Noissue - Add some more JavaScript coding guidlines (Closed)
Left Patch Set: Created Jan. 16, 2017, 7:04 a.m.
Right Patch Set: Addressed further feedback Created Jan. 18, 2017, 11:06 a.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
« no previous file with change/comment | « no previous file | no next file » | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
1 title=Coding Style 1 title=Coding Style
2 2
3 3
4 <h2 id="precedence">{{s1 Precedence}}</h2> 4 <h2 id="precedence">{{s1 Precedence}}</h2>
5 5
6 <p>{{s2 The Adblock Plus coding style is based on others. Our rules (both genera l and language specific) always overrule referenced style guides. The language s pecific sections overrule the general section.}}</p> 6 <p>{{s2 The Adblock Plus coding style is based on others. Our rules (both genera l and language specific) always overrule referenced style guides. The language s pecific sections overrule the general section.}}</p>
7 7
8 <h2 id="consistency">{{s3 Consistency}}</h2> 8 <h2 id="consistency">{{s3 Consistency}}</h2>
9 9
10 <p>{{s4 Consistency is most important: Be consistent within functions, files, mo dules and projects. Making existing code conform with this style guide is fine, but it should happen in dedicated commits, preferably for a whole module or proj ect at once.}}</p> 10 <p>{{s4 Consistency is most important: Be consistent within functions, files, mo dules and projects. Making existing code conform with this style guide is fine, but it should happen in dedicated commits, preferably for a whole module or proj ect at once.}}</p>
11 11
12 <h2 id="general">{{s5 General}}</h2> 12 <h2 id="general">{{s5 General}}</h2>
13 <ul> 13 <ul>
14 <li>{{s6 Follow the Mozilla Coding Style's <a href="https://developer.mozilla. org/en-US/docs/Developer_Guide/Coding_Style#General_C.2FC.2B.2B_Practices">gener al practices</a> and its <a href="https://developer.mozilla.org/en-US/docs/Devel oper_Guide/Coding_Style#Naming_and_Formatting_code">naming and formatting rules< /a>.}}</li> 14 <li>{{s6 Follow the Mozilla Coding Style's <a href="https://developer.mozilla. org/en-US/docs/Developer_Guide/Coding_Style#General_C.2FC.2B.2B_Practices">gener al practices</a> and its <a href="https://developer.mozilla.org/en-US/docs/Devel oper_Guide/Coding_Style#Naming_and_Formatting_code">naming and formatting rules< /a>.}}</li>
15 <li>{{s8 All files should have a <a href="license-headers">license header</a>, but no mode line comments.}}</li> 15 <li>{{s8 All files should have a <a href="license-headers">license header</a>, but no mode line comments.}}</li>
16 <li>{{s9 Newline at end of file, otherwise no trailing whitespace.}}</li> 16 <li>{{s9 Newline at end of file, otherwise no trailing whitespace.}}</li>
17 <li>{{s10 Lines can be longer than the limit, if limiting line length would hu rt readability in a particular case.}}</li> 17 <li>{{s10 Lines can be longer than the limit, if limiting line length would hu rt readability in a particular case.}}</li>
18 <li>{{s11 Opening braces always go on their own line.}}</li> 18 <li>{{s11 Opening braces always go on their own line.}}</li>
19 <li>{{s12 No hungarian notation, no special variable name prefixes or suffixes denoting type or scope. All variable names start with a lower case letter.}}</l i> 19 <li>{{s12 No hungarian notation, no special variable name prefixes or suffixes denoting type or scope. All variable names start with a lower case letter.}}</l i>
20 <li>{{s13 Don't comment code out, delete it.}}</li> 20 <li>{{s13 Don't comment code out, delete it.}}</li>
21 <li>{{todo-comments Don't add TODO comments, create an <a href="https://issues .adblockplus.org">issue</a> and reference it in a comment instead.}}</li> 21 <li>{{todo-comments Don't add TODO comments, create an <a href="https://issues .adblockplus.org">issue</a> and reference it in a comment instead.}}</li>
22 </ul> 22 </ul>
23 23
24 <h2 id="javascript">{{s14 JavaScript}}</h2> 24 <h2 id="javascript">{{s14 JavaScript}}</h2>
25 <ul> 25 <ul>
26 <li>{{s15 Follow the Mozilla Coding Style's <a href="https://developer.mozilla .org/en-US/docs/Developer_Guide/Coding_Style#JavaScript_practices">JavaScript pr actices</a>.}}</li> 26 <li>{{s15 Follow the Mozilla Coding Style's <a href="https://developer.mozilla .org/en-US/docs/Developer_Guide/Coding_Style#JavaScript_practices">JavaScript pr actices</a>.}}</li>
27 <li>{{s16 Opening braces of object literals in don't go on their own line when that would cause a syntax error.}}</li> 27 <li>{{javascript-strict Always use <a href="https://developer.mozilla.org/docs /Web/JavaScript/Reference/Strict_mode">strict mode</a>.}}</li>
28 <li>{{javascript-this Use <code>bind()</code> (or arrow functions) to ensure t he desired value of the <code>this</code> variable, don’t use temporary variable s as a replacement.}}</li> 28 <li>{{javascript-opening-braces Opening braces of object literals don't go on their own line when that would cause a syntax error.}}</li>
Sebastian Noack 2017/01/16 17:07:14 For code snippets please also use the <fix> tag, s
Sebastian Noack 2017/01/16 17:07:14 I think this should go below the rule for arrow fu
kzar 2017/01/17 06:48:51 Done.
kzar 2017/01/17 06:48:51 Done.
29 <li>{{javascript-if-else-braces When an <code><fix>if</fix></code> statement, an <code><fix>else</fix></code> statement or a loop spans over more than one lin e always enclose it with braces. When an <code><fix>if</fix></code> or <code><fi x>else</fix></code> statement uses braces the opposing block should do too.}}</l i>
29 <li>{{s18 In classes, prefix private functions with a single underscore to mak e them pseudo-private.}}</li> 30 <li>{{s18 In classes, prefix private functions with a single underscore to mak e them pseudo-private.}}</li>
30 <li>{{javascript-let Use <code>let</code> or better yet <code>const</code> for variable definitions where possible instead of <code>var</code>.}}</li> 31 <h3 id="javascript-modern">{{javascript-es6 Modern JavaScript (where supported )}}</h3>
Sebastian Noack 2017/01/16 17:07:14 I find this rule a little too vague. How about:
kzar 2017/01/17 06:48:51 Done.
31 <li>{{javascript-arrow Use the <a href="https://developer.mozilla.org/docs/Web /JavaScript/Reference/Functions/Arrow_functions">arrow function</a> syntax for a nonymous functions when the binding of <code>this</code> and <code>arguments</co de> is not required.}}</li> 32 <ul>
Sebastian Noack 2017/01/16 17:07:14 Arrow functions automatically bind "this". So it s
kzar 2017/01/17 06:48:51 Well I think arrow functions simply close over the
32 <li>{{javascript-method Use the <a href="https://developer.mozilla.org/docs/We b/JavaScript/Reference/Functions/Method_definitions">method definition</a> synta x where possible, even in preference to arrow functions.}}</li> 33 <li>{{javascript-block-scoping Always use <a href="https://developer.mozilla .org/docs/Web/JavaScript/Reference/Statements/block#With_let_and_const">block-sc oping</a> (<code><fix>let</fix></code> / <code><fix>const</fix></code>), except when sharing global variables between scripts cannot be avoided.}}</li>
Sebastian Noack 2017/01/16 17:07:14 I don't like how we overrule another rule here (i.
Sebastian Noack 2017/01/16 17:07:14 "method definition" is ambiguous as the old syntax
kzar 2017/01/17 06:48:51 Done.
kzar 2017/01/17 06:48:51 Done.
kzar 2017/01/17 06:48:51 Done.
33 <li>{{javascript-strict Where possible always use <a href="https://developer.m ozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode">strict mode</a>.}}</ li> 34 <li>{{javascript-const Use <code><fix>const</fix></code> for constant expres sions that could as well have been inlined (e.g. static parameters, imported mod ules, etc.).}}</li>
Sebastian Noack 2017/01/16 17:07:14 Why the "where possible" restriction? Is there any
kzar 2017/01/17 06:48:51 Done.
34 <li>{{javascript-if-else-braces When an <code>if</code>, <code>else</code> or <code>loop</code> statement spans over more than one line always enclose it with braces. When an <code>if</code> or <code>else</code> statement uses braces the other half should do too.}}</li> 35 <li>{{javascript-arrow Use the <a href="https://developer.mozilla.org/docs/W eb/JavaScript/Reference/Functions/Arrow_functions">arrow function</a> syntax whe n passing anonymous functions that don't require the binding of their own <code> <fix>this</fix></code> or <code><fix>arguments</fix></code> variables.}}</li>
Sebastian Noack 2017/01/16 17:07:14 "other half" doesn't sound too technically accurra
Sebastian Noack 2017/01/16 17:07:14 "loop" isn't a literal code snippet, so it shouldn
kzar 2017/01/17 06:48:51 Done.
kzar 2017/01/17 06:48:52 Done.
35 <li>{{javascript-for-of When iterating through Arrays and similar prefer <code >for (let item of array)</code> loops to traditional <code>for (let i=0; i < arr ay.length; i++)</code> or even <code>array.forEach(...)</code> unless there is a good reason to do otherwise.}}</li> 36 <li>{{javascript-method Use the <a href="https://developer.mozilla.org/docs/ Web/JavaScript/Reference/Functions/Method_definitions">method definition shortha nd</a> syntax when defining methods on an object.}}</li>
Sebastian Noack 2017/01/16 17:07:14 I think we are really only talking about arrays he
kzar 2017/01/17 06:48:51 Done.
36 <li>{{javascript-object-proto Use <code>Object.create(null)</code> instead of <code>{__proto__: null}</code> and otherwise avoid the use of <code>__proto__</c ode>.}}</li> 37 <li>{{javascript-for-of When iterating over arrays in the canonical order us e the new <a href="https://developer.mozilla.org/docs/Web/JavaScript/Reference/S tatements/for...of"><code><fix>for..of</fix></code></a> syntax if the loop index isn't required. (Rather than using <code><fix>for(;;)</fix></code> or the <code ><fix>forEach()</fix></code> method.)}}</li>
Sebastian Noack 2017/01/16 17:07:14 Well, by now, you should rather use Map or Set obj
kzar 2017/01/17 06:48:52 Done.
saroyanm 2017/01/18 11:27:02 Detail: Please add full stop in the end of the lin
Sebastian Noack 2017/01/18 11:38:08 Well, if you put the whole sentence into parenthes
saroyanm 2017/01/18 11:49:49 Learned something today. Fare enough.
38 <li>{{javascript-map-set Use the <a href="https://developer.mozilla.org/docs /Web/JavaScript/Reference/Global_Objects/Map"><code><fix>Map</fix></code></a> or <a href="https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Obj ects/Set"><code><fix>Set</fix></code></a> objects, rather than misusing plain ob jects, when you need a hash table.}}</li>
39 </ul>
37 </ul> 40 </ul>
38 41
39 <h2 id="html-css">{{html-css HTML and CSS}}</h2> 42 <h2 id="html-css">{{html-css HTML and CSS}}</h2>
40 <ul> 43 <ul>
41 <li>{{html-css-general Follow the <a href="https://google.github.io/styleguide /htmlcssguide.xml">Google HTML/CSS Style Guide</a>.}}</li> 44 <li>{{html-css-general Follow the <a href="https://google.github.io/styleguide /htmlcssguide.xml">Google HTML/CSS Style Guide</a>.}}</li>
42 <li>{{html-css-quotation Use double over single quotation marks.}}</li> 45 <li>{{html-css-quotation Use double over single quotation marks.}}</li>
43 <li>{{html-css-optional-tag Don't omit optional HTML tags.}}</li> 46 <li>{{html-css-optional-tag Don't omit optional HTML tags.}}</li>
44 <li>{{html-css-hexadecimal CSS color values should be specified in hexadecimal where possible.}}</li> 47 <li>{{html-css-hexadecimal CSS color values should be specified in hexadecimal where possible.}}</li>
45 <li>{{html-css-shorthand CSS shorthand properties usage is optional.}}</li> 48 <li>{{html-css-shorthand CSS shorthand properties usage is optional.}}</li>
46 <li>{{html-css-order CSS rule declaration order should follow the <a href="htt ps://make.wordpress.org/core/handbook/best-practices/coding-standards/css/#prope rty-ordering">WordPress CSS Coding Standards</a>.}}</li> 49 <li>{{html-css-order CSS rule declaration order should follow the <a href="htt ps://make.wordpress.org/core/handbook/best-practices/coding-standards/css/#prope rty-ordering">WordPress CSS Coding Standards</a>.}}</li>
(...skipping 35 matching lines...) Expand 10 before | Expand all | Expand 10 after
82 <ul> 85 <ul>
83 <li>{{objc-conventions Follow Apple's <a href="https://developer.apple.com/lib rary/ios/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/Conventions/Co nventions.html">conventions</a>}}.</li> 86 <li>{{objc-conventions Follow Apple's <a href="https://developer.apple.com/lib rary/ios/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/Conventions/Co nventions.html">conventions</a>}}.</li>
84 </ul> 87 </ul>
85 88
86 <h2 id="puppet">{{s31 Puppet}}</h2> 89 <h2 id="puppet">{{s31 Puppet}}</h2>
87 <ul> 90 <ul>
88 <li>{{s32 Follow the <a href="http://docs.puppetlabs.com/guides/style_guide.ht ml">Puppet Style Guide</a>.}}</li> 91 <li>{{s32 Follow the <a href="http://docs.puppetlabs.com/guides/style_guide.ht ml">Puppet Style Guide</a>.}}</li>
89 <li>{{s33 Opening braces don't go on their own line.}}</li> 92 <li>{{s33 Opening braces don't go on their own line.}}</li>
90 <li>{{s34 Arrows should not be aligned.}}</li> 93 <li>{{s34 Arrows should not be aligned.}}</li>
91 </ul> 94 </ul>
LEFTRIGHT
« no previous file | no next file » | Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Toggle Comments ('s')

Powered by Google App Engine
This is Rietveld