Spaces inside of () is the MW convention, inside of [] they're indeed unneeded.
On Thu, Nov 20, 2014 at 4:03 PM, Bahodir Mansurov <[email protected]> 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 <[email protected]> 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-L257 > > 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 > <[email protected]> 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 <[email protected]> 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 > [email protected] > https://lists.wikimedia.org/mailman/listinfo/mobile-l > > > > > _______________________________________________ > Mobile-l mailing list > [email protected] > https://lists.wikimedia.org/mailman/listinfo/mobile-l > > -- Best regards, Max Semenik ([[User:MaxSem]])
_______________________________________________ Mobile-l mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mobile-l
