Page MenuHomePhabricator

Growth Team repos: require trailing commas
Closed, ResolvedPublic

Description

Per consensus among the engineers in the team, we want for our repositories a consistent code style that requires trailing commas (a.k.a. dangling commas) in both PHP and JavaScript / TypeScript.

In PHP, this will use the sniff from T222042: Add sniff to require trailing commas in multiline arrays.

Event Timeline

Change #1162020 had a related patch set uploaded (by Michael Große; author: Michael Große):

[mediawiki/extensions/WikiLove@master] style: require trailing commas in js code

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

Change #1162021 had a related patch set uploaded (by Michael Große; author: Michael Große):

[mediawiki/extensions/WikiLove@master] style: require trailing commas in PHP code

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

Change #1162033 had a related patch set uploaded (by Michael Große; author: Michael Große):

[mediawiki/extensions/Thanks@master] style: require trailing commas in js code

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

Change #1162034 had a related patch set uploaded (by Michael Große; author: Michael Große):

[mediawiki/extensions/Thanks@master] style: require trailing commas in PHP code

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

Change #1162048 had a related patch set uploaded (by Michael Große; author: Michael Große):

[mediawiki/extensions/GuidedTour@master] style: require trailing commas in js code

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

Change #1162049 had a related patch set uploaded (by Michael Große; author: Michael Große):

[mediawiki/extensions/GuidedTour@master] style: require trailing commas in PHP code

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

Change #1162020 merged by jenkins-bot:

[mediawiki/extensions/WikiLove@master] style: require trailing commas in js code

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

Change #1162048 merged by jenkins-bot:

[mediawiki/extensions/GuidedTour@master] style: require trailing commas in js code

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

Change #1162021 merged by jenkins-bot:

[mediawiki/extensions/WikiLove@master] style: require trailing commas in PHP code

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

Change #1162049 merged by jenkins-bot:

[mediawiki/extensions/GuidedTour@master] style: require trailing commas in PHP code

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

Change #1162033 merged by jenkins-bot:

[mediawiki/extensions/Thanks@master] style: require trailing commas in js code

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

Change #1162034 merged by jenkins-bot:

[mediawiki/extensions/Thanks@master] style: require trailing commas in PHP code

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

Change #1183089 had a related patch set uploaded (by Michael Große; author: Michael Große):

[mediawiki/extensions/Echo@master] style: require trailing commas in PHP code

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

Change #1183090 had a related patch set uploaded (by Michael Große; author: Michael Große):

[mediawiki/extensions/Echo@master] style: require trailing commas in js code

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

Change #1183089 merged by jenkins-bot:

[mediawiki/extensions/Echo@master] style: require trailing commas in PHP code

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

Change #1183090 merged by jenkins-bot:

[mediawiki/extensions/Echo@master] style: require trailing commas in js code

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

Change #1190616 had a related patch set uploaded (by Michael Große; author: Michael Große):

[mediawiki/extensions/GrowthExperiments@master] style: require trailing commas in PHP code

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

Change #1190621 had a related patch set uploaded (by Michael Große; author: Michael Große):

[mediawiki/extensions/GrowthExperiments@master] style: require trailing commas in js code

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

Change #1190616 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] style: require trailing commas in PHP code

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

Change #1190621 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] style: require trailing commas in js code

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

This should cover all our repos. It is purely about style change, no point in doing QA on this. Resolving.

Esanders subscribed.

I'm not sure having a such a significant divergence from https://www.mediawiki.org/wiki/Manual:Coding_conventions/JavaScript for just a handful of repos is a good idea. It makes code less portable and searchable, and has added a lot of noise to the revision history (as a non-whitespace change). There is a reason we try to keep code style consistent across projects. This task is also lacking any rationale for such a significant change.

I'm happy to have the conversation about evolving the MediaWiki code style as a whole in that aspect, the main reasons for the convention to forbid trailing commas have been obsolete for a long time now.

However, the social process of changing a style like this top-down tends to be slow and tiresome. And while a consistent code style across projects is a valid goal overall, it should not prevent teams who want to move faster towards adopting better standards from doing so. There is also a reason why the teams themselves ultimately have the last say about which code style they want to have in their repositories.

(I'm removing the component-specific tags because this task is now just noise on those boards.)

However, the social process of changing a style like this top-down tends to be slow and tiresome.

Yes, but we should still try. We have made some pretty significant changes in eslint-config-wikimedia over the past few years, by consensus.

it should not prevent teams who want to move faster towards adopting better standards from doing so

I don't yet see an agreement on which style is "better", but that conversation should be had before mass-migrating repos.

There is also a reason why the teams themselves ultimately have the last say about which code style they want to have in their repositories.

Teams also have a responsibility to the rest of the MediaWiki ecosystem, not just to their personal preferences around formatting.

However, the social process of changing a style like this top-down tends to be slow and tiresome.

Yes, but we should still try. We have made some pretty significant changes in eslint-config-wikimedia over the past few years, by consensus.

I see us coming to a consensus about introducing new rules for formatting questions that were not yet defined, but do you have an example for reversing an existing rule that would affect so many lines of code? In this case, I think getting there bottom-up is more realistic than top-down, if at all.

it should not prevent teams who want to move faster towards adopting better standards from doing so

I don't yet see an agreement on which style is "better", but that conversation should be had before mass-migrating repos.

There is also a reason why the teams themselves ultimately have the last say about which code style they want to have in their repositories.

Teams also have a responsibility to the rest of the MediaWiki ecosystem, not just to their personal preferences around formatting.

Having consistency around formatting in the MediaWiki ecosystem is a nice-to-have goal in general. For the specific case of trailing commas, the baseline situation is already much more mixed. We already have no rule in either direction for them in PHP, so there is no consensus to begin with in half of our code-base. And seeing how the conversation went for a much less disruptive introduction of such a rule in T397111 and in the parent task, I don't think that there is much point in trying to start this bigger conversation.
If at all, there might be potential for a consensus to drop that rule from our js code-style without replacement, effectively leaving it more explicitly up to the teams to decide what formatting they would like to have by stopping to provide a recommendation. I think that would at least be a small step in the right direction; what are your thoughts? If one were to start that conversation, how would one do that? A phab task proposing dropping the comma-dangle rule from eslint-config-wikimedia and then highlighting it in the relevant places?