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
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
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
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
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
Do you know why there are spaces inside () but not inside []. Just curious.
On Nov 20, 2014, at 7:07 PM, Max Semenik maxsem.wiki@gmail.com 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 <bmansurov@wikimedia.org mailto: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 mailto:jrobson@wikimedia.org> wrote:
Follow up If I run `make jsbeautify` now then https://gist.github.com/jdlrobson/a05ddad00175ebceac68 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... 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 <bmansurov@wikimedia.org mailto: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 mailto: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 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 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 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 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 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 https://github.com/beautify-web/js-beautify [2] https://github.com/jscs-dev/node-jscs https://github.com/jscs-dev/node-jscs
Mobile-l mailing list Mobile-l@lists.wikimedia.org mailto:Mobile-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mobile-l https://lists.wikimedia.org/mailman/listinfo/mobile-l
Mobile-l mailing list Mobile-l@lists.wikimedia.org mailto:Mobile-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mobile-l https://lists.wikimedia.org/mailman/listinfo/mobile-l
-- Best regards, Max Semenik ([[User:MaxSem]])
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" bmansurov@wikimedia.org escribió:
Do you know why there are spaces inside () but not inside []. Just curious.
On Nov 20, 2014, at 7:07 PM, Max Semenik maxsem.wiki@gmail.com 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 <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
-- Best regards, Max Semenik ([[User:MaxSem]])
Mobile-l mailing list Mobile-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mobile-l
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_len...
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 < jhernandez@wikimedia.org> 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" bmansurov@wikimedia.org escribió:
Do you know why there are spaces inside () but not inside []. Just curious.
On Nov 20, 2014, at 7:07 PM, Max Semenik maxsem.wiki@gmail.com 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 < 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
-- Best regards, Max Semenik ([[User:MaxSem]])
Mobile-l mailing list Mobile-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mobile-l
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" jhernandez@wikimedia.org 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_len...
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 < jhernandez@wikimedia.org> 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" bmansurov@wikimedia.org escribió:
Do you know why there are spaces inside () but not inside []. Just
curious.
On Nov 20, 2014, at 7:07 PM, Max Semenik maxsem.wiki@gmail.com 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 < 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
-- Best regards, Max Semenik ([[User:MaxSem]])
Mobile-l mailing list Mobile-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mobile-l