Spaces inside of () is the MW convention, inside of [] they're indeed unneeded.
On Thu, Nov 20, 2014 at 4:03 PM, Bahodir Mansurov bmansurov@wikimedia.org wrote:
I honestly think that we should do away with spaces inside () and []. It puts an end to ugly code like this:
if ( $viewportMeta[ 0 ] && ( isIPhone4 || isIPhone5 ) ) {
compare with this (so much nicer) if ($viewportMeta[0] && (isIPhone4 || isIPhone5)) {
By removing space we are no longer stressing () and [], the focus shifts to other parts of code for easier reading.
On Nov 20, 2014, at 3:19 PM, Jon Robson jrobson@wikimedia.org wrote:
Follow up If I run `make jsbeautify` now then https://gist.github.com/jdlrobson/a05ddad00175ebceac68 is the result.
Outstanding actions:
- Need input on rest of the team about which of delete
this.cache[title]; or delete this.cache[ title ]; is the preferred standard.
- You'll notice jsbeautify and inlne comments need to be ironed out.
For example:
https://gist.github.com/jdlrobson/a05ddad00175ebceac68#file-gistfile1-diff-L...
Apart from the above 2 jsbeautify makes some adjustments to our whitspace which I guess we'll just have to live with.
Please can everyone else let me know how they think we should proceed?
On Wed, Nov 19, 2014 at 4:12 AM, Bahodir Mansurov bmansurov@wikimedia.org wrote:
As for # Rule 4, it makes sense to add spaces inside square brackets. The reasoning is the same as why we add spaces inside parenthesis.
On Nov 18, 2014, at 12:28 PM, Jon Robson jrobson@wikimedia.org wrote:
I explored running jsbeautify [1] on the MobileFrontend codebase and looked at how the output differs from the current styling. It introduces various rules that MobileFrontend is currently not adhering too. MobileFrontend already uses jscs [2] so we want to make sure the outputs of both are compatible. Here is my report on that with the recommendation that we should use it.
#Rule 1: object properties defined on a single line. e.g. { foo: 2, bar: 3 } NOT { foo: 2, bar: 3 }
I think this would be a good idea to adopt. I will explore if jscs can enforce this.
# Rule 2: variables that are initialised must be followed by a new line (although I noted a few odd cases e.g. in Page.js after a "?:" expression and /MobileWebClickTracking.js e.g. var Foo, bar = $( 'div' ), Baz, Bar;
not: var Foo, bar = $( 'div' ), Baz, Bar;
This will be fixed if I implement https://trello.com/c/0dkx0ldL
# Rule 3: All chained events should be indented e.g. foo() .bar();
not foo(). bar();
Seems like a no brainer. One that happens naturally most of the time.
# Rule 4: Spacing in object parameters e.g. foo[ 1 ] [ 1, 2, 3, 4 ]
not: foo[1] [1, 2, 3, 4]
This is different to MediaWiki coding conventions but I can implement https://github.com/beautify-web/js-beautify/issues/424 to give us this. We seem a bit inconsistent ourselves with this convention - let me know how you think this rule should work in our codebase...
# Rule 5: New line after variable declarations
var x, y, z;
z();
not: var x, y, z; z();
Also: function foo() {}
function bar() {}
not: function foo() {} function bar() {}
Seems like a no brainer and shouldn't introduce any major issues with code review.
# Rule 6: Comments must respect the current indentation level
if () { ... // If i is 5 we do something special } else if ( i === 5 ){
} becomes if () { ... // If i is 5 we do something special } else if ( i === 5 ){
} We'll have to think about what to do with these comments but I don't think this should be a blocker to using jsbeautify. It will only pop up occasionally.
# Rule 7: On long if statements the curly brace must be indented. And the first condition should be on the same line
if ( enableToc || ... .... && baz ) {
not: if ( enableToc || ... .... && baz ) {
Again I'm not sure if this will happen too often. This to me is a sign that we should be using functions rather than long if statements personally. Again not a blocker.
Actions:
- Implement https://github.com/beautify-web/js-beautify/issues/424
- You guys need to advise me on how we should handle square brackets
in our codebase in such a way we respect MediaWiki coding conventions and come up with a consistent style we are happy with and can adhere to.
- I'll implement https://trello.com/c/0dkx0ldL in some form
- I'll explore if jscs can support objects defined on new line
- When the above are done I recommend we turn on jsbeautify for the
project.
I've setup a tracking card for the above work: https://trello.com/c/5btWf2JN/90-snakes-on-a-plane and will be looking at these problems today.
[1] https://github.com/beautify-web/js-beautify [2] https://github.com/jscs-dev/node-jscs
Mobile-l mailing list Mobile-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mobile-l
Mobile-l mailing list Mobile-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mobile-l