Change 233934 had a related patch set uploaded (by Jonas Kress (WMDE)):
jscs: require space after all keywords, align with wikimedia preset

https://gerrit.wikimedia.org/r/233934

I'm for aligning with wikimedia core.

Note that the code style we are messing around with here was intentionally created like that in November 2013. The team changed a lot since then and we may agree that this is reason enough to have this discussion.

This is what I would like to do:

1. If the relevant developers actually working on JavaScript code (this is Adrian, Jonas and me) agree, I'm all positive to change the rule for control structures to have an enforced space: for (), if (), switch (), while (), also including catch (). https://www.mediawiki.org/wiki/Wikibase/Coding_conventions must be changed then.
2. A strong no to function () with a space. This contradicts the reasoning behind this "space vs. no space" rule: function declarations and calls never have a space. In PHP public function foo( $param ) does not have a space, calling $this->foo( $param ) does not have a space, so why should function( $param ) have a space? Same in JavaScript, function foo( param ) does not have a space, calling this.foo( param ) does not have a space, so why should foo = function( param ) have a space?

Does https://github.com/jscs-dev/node-jscs/blob/master/presets/wikimedia.json contain a rule for the function() vs. function () ? I not we should add one upstream.

In T110811#1606300, @JanZerebecki wrote:

Does https://github.com/jscs-dev/node-jscs/blob/master/presets/wikimedia.json contain a rule for the function() vs. function () ? I not we should add one upstream.

"requireSpaceAfterKeywords": true implies "function". Wikibase's .jscsrc overwrites it, that's what https://gerrit.wikimedia.org/r/233934 is all about.

Now I get it, I was confused by the setting it to true vs an array of words.

In T110811#1606110, @thiemowmde wrote:

In PHP public function foo( $param ) does not have a space, calling $this->foo( $param ) does not have a space, so why should function( $param ) have a space? Same in JavaScript, function foo( param ) does not have a space, calling this.foo( param ) does not have a space, so why should foo = function( param ) have a space?

But in your examples there is always a space after function.

In T110811#1606110, @thiemowmde wrote:

A strong no to function () with a space. This contradicts the reasoning behind this "space vs. no space" rule: function declarations and calls never have a space. In PHP public function foo( $param ) does not have a space, calling $this->foo( $param ) does not have a space, so why should function( $param ) have a space? Same in JavaScript, function foo( param ) does not have a space, calling this.foo( param ) does not have a space, so why should foo = function( param ) have a space?

function( param ) could be mistaken for a declaration of a function named "function".
MediaWiki core uses "function" + " " + function name + "(" ecc. and the name of an anonymous function is obviously empty. Also in PHP, because I can find much more occurrences of "function (" than "function(".

This is just to clarify that I had no intention whatsoever of changing established conventions. I can help you set jscs up for whatever rules you want to follow.

I don't feel strongly about the coding style change. In the long run it probably makes sense to have Wikibase PHP and Wikibase JS as well as Wikibase JS and MediaWiki JS follow the same coding style. I'm not looking forward to the merge conflicts this will give us, though.

long run win > short term pain? :)

My suggestion: Let's stick with https://www.mediawiki.org/wiki/Wikibase/Coding_conventions#JavaScript_style_policies. Thoughts were put into this by the JS developers in our team back then. I'm not against changing it at some point, but we should carefully consider this and sit together.
What I find a good idea definitely is the suggestion by @Ricordisamoa to set up proper jscs rules for whatever conventions we end up using.

In T110811#1640976, @Tobi_WMDE_SW wrote:

My suggestion: Let's stick with https://www.mediawiki.org/wiki/Wikibase/Coding_conventions#JavaScript_style_policies. Thoughts were put into this by the JS developers in our team back then. I'm not against changing it at some point, but we should carefully consider this and sit together.

This is what this ticket is about. Let's decide. Yes back then there were reasons but that's not stopping us from reevaluating.

AFAIK the ticket was created based on a patch that was uploaded before the topic was discussed and by someone who was not aware of our Wikibase JS coding conventions. I get from the conversation on this ticket that for the JS developers there is actually no problem with sticking to that convention so I suggested to not waste more time on this topic for now.

committed for Wikidata-Sprint-2015-09-29: @thiemowmde and @Jonas and @JanZerebecki sit together and decide on this.

Change 242615 had a related patch set uploaded (by Thiemo Mättig (WMDE)):
Enable JSCS rule requireSpaceAfterKeywords

https://gerrit.wikimedia.org/r/242615

Patches for review:

• https://gerrit.wikimedia.org/r/242615
• https://github.com/wmde/ValueView/pull/195

Change 242615 merged by jenkins-bot:
Enable JSCS rule requireSpaceAfterKeywords in "google" style

https://gerrit.wikimedia.org/r/242615

The only thing we could not agree on is requireSpaceAfterKeywords for function.

Change 242927 had a related patch set uploaded (by JanZerebecki):
JSCS: requre space also after case, void, with, typeof

https://gerrit.wikimedia.org/r/242927

Change 243083 had a related patch set uploaded (by Ricordisamoa):
build: Enable jscs jsDoc rule 'checkTypes' and make pass

https://gerrit.wikimedia.org/r/243083

Change 243090 had a related patch set uploaded (by Ricordisamoa):
build: Enable jscs jsDoc rule 'checkParamNames' and make pass

https://gerrit.wikimedia.org/r/243090

Change 243097 had a related patch set uploaded (by Ricordisamoa):
build: Enable jscs jsDoc rule 'requireReturnTypes' and make pass

https://gerrit.wikimedia.org/r/243097

Change 243083 merged by jenkins-bot:
build: Enable jscs jsDoc rule 'checkTypes' and make pass

https://gerrit.wikimedia.org/r/243083

Change 243097 merged by jenkins-bot:
build: Enable jscs jsDoc rule 'requireReturnTypes' and make pass

https://gerrit.wikimedia.org/r/243097

Change 243090 merged by jenkins-bot:
build: Enable jscs jsDoc rule 'checkParamNames' and make pass

https://gerrit.wikimedia.org/r/243090

Change 242927 merged by jenkins-bot:
JSCS: require space also after case, void, with, typeof

https://gerrit.wikimedia.org/r/242927

Shall I abandon https://gerrit.wikimedia.org/r/233934 then?

Shall I abandon https://gerrit.wikimedia.org/r/233934 then?

In my opinion, yes. At least for now. We caused quite some trouble and rebases to this point. Would be nice to focus on more productive stuff. Don't get me wrong, I'm thankful for what you did. The fact that an if() in JavaScript and an if () in PHP always looked different always bugged me.

In https://gerrit.wikimedia.org/r/242615 I collected a few more reasons for sticking with function():

• It's "official" in other JSCS presets, e.g. the "google" preset.
• JSCS itself uses the "google" preset.
• Even other big Wikimedia projects like Parsoid and Flow do it like we do.
• MediaWiki core's PHP uses a mixture of function() and function () with no clear preference and obviously no PHPCS set up to enforce anything. I find this relevant because I consider consistency across languages important.

Change 233934 abandoned by Ricordisamoa:
jscs: require space after "function", align with wikimedia preset

Reason:
T110811#1696595

https://gerrit.wikimedia.org/r/233934

For the record, this is what we decided to left out for now:

• We decided to neither enforce function() nor function () for now.
• The components Wikibase-DataModel-JavaScript, Wikibase-JavaScript-Api and Wikibase-Serialization-JavaScript do not have JSCS set up at the moment. If anybody wants to change that, feel free.