I think the most important thing to clarify would be the brackets w/ spaces thing. I'm personally fine with whatever it is that is tool supported (without spaces will need a pull request to js-beautify).
Other than that it looks ready to go. Also about the tooling infrastructure, we need: - Add filter (adding files gets them beautified transparently) (something like https://gerrit.wikimedia.org/r/#/c/173026/) - This would make every new js change be beautified in the repo when adding before commiting, removing most of jscs warnings bothering us - Grunt task for beautifying files & link make jsbeautify to that El 21/11/2014 11:38, "Joaquin Oltra Hernandez" <[email protected]> escribió: > About if statements boolean conditions, note that JS beautify beautifies > properly all of them: > > if ( true || > ( false || true ) && > ( true && false ) ) { > console.log( true ); > } > > if ( true || > ( false || true ) && > ( true && false ) > ) { > console.log( true ); > } > > if ( > true || > ( false || true ) && > ( true && false ) > ) { > console.log( true ); > } > > Being the second one the recommended by the mediawiki conventions > https://www.mediawiki.org/wiki/Manual:Coding_conventions/JavaScript#Line_length > > In this case, the beautifier will help a bit, but the work of converting > if number 1 and 3 to version 2 is on us. > > On Fri, Nov 21, 2014 at 10:05 AM, Joaquin Oltra Hernandez < > [email protected]> wrote: > >> In with you baha, mostly JS conventions around the world do not use >> spaces around everything, but wikimedia conventions do, and we shouldn't >> ignore them. In the end readability is subjective, and the ones who get >> there first have the upper hand. >> >> The good part is that with the js beautify effort we won't have to worry >> about, at least, the format. Code will be automatically formated before >> linting and adding / committing, so that will improve the consistency of >> the codebase coding style, and remove friction for us and new developers >> when contributing code. >> >> Personally I think there is nothing more annoying when contributing than >> being nitpicked about the format when it could/should be an automatic thing. >> El 21/11/2014 1:14, "Bahodir Mansurov" <[email protected]> >> escribió: >> >> Do you know why there are spaces inside () but not inside []. Just >>> curious. >>> >>> On Nov 20, 2014, at 7:07 PM, Max Semenik <[email protected]> wrote: >>> >>> 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 >>> >>> >
_______________________________________________ Mobile-l mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mobile-l
