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
