Page MenuHomePhabricator

General cleanup of initialize settings
Open, LowestPublic

Description

As a follow up to T231040: Clean up `wgRateLimits` to remove unneeded entries, T231041: Clean up `groupOverrides` to remove unneeded entries, and T230797: Clean up `wgNamespacesToBeSearchedDefault` to remove unneeded entries, there is some more general cleanup that can be done. Rather than doing this piecemeal, I've uploaded a patch that should cover most of it.

Details

Related Gerrit Patches:
operations/mediawiki-config : masterPartial cleanup of InitializeSettings
operations/mediawiki-config : masterPartial cleanup of InitialiseSettings
operations/mediawiki-config : masterPartial cleanup of InitialiseSettings
operations/mediawiki-config : masterGeneral cleanup of initialize settings

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 25 2019, 10:10 PM
DannyS712 updated the task description. (Show Details)Aug 25 2019, 10:10 PM

Change 532280 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[operations/mediawiki-config@master] General cleanup of initialise settings

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

DannyS712 updated the task description. (Show Details)Aug 26 2019, 1:36 AM
DannyS712 updated the task description. (Show Details)
DannyS712 added a project: User-Urbanecm.
DannyS712 moved this task from Backlog to To deploy on the User-Urbanecm board.

Change 532280 abandoned by DannyS712:
General cleanup of initialize settings

Reason:
Fails to merge cleanly with recent changes, will try again later with smaller-scale patches to speed up deployment and reduce the odds of this happening again

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

Thanks Danny. Frel free to move it back once there's something actionable for me (review or deployment, or both :)).

Thanks Danny. Frel free to move it back once there's something actionable for me (review or deployment, or both :)).

Thanks. Other than Krinkle's comment, which I resolved, was there any other issue you noticed with the final proposal in PS11? The reason I abandoned this was that too many other things were merged in the meanwhile. In theory, this patch should be uncontroversial, since it doesn't change the result of the code.

Dcljr added a subscriber: Dcljr.Sep 2 2019, 9:29 AM

For what it's worth (which may well be nothing), I think changes such as replacing:

'+brwiki' => [
	'Discussion_Wikipedia' => NS_PROJECT_TALK,
],

with:

'+brwiki' => [ 'Discussion_Wikipedia' => NS_PROJECT_TALK, ],

actually make the file harder to use, since the latter formatting would have to be changed back to the former to add more entries/settings for the same wiki. (In this case, this is under 'wgNamespaceAliases', but the same goes for 'wgExtraNamespaces' and other places in the file.)

This also adversely affects readability (IMO), since most of the wikis do have multiple entries/settings (sorry, I don't know the proper terminology) for those variables, and only a few have a single one.

I would recommend not making this kind of change unless the vast majority (if not all) of the entries can use the single-line format (such the changes made to 'wgProofreadPageNamespaceIds').

For what it's worth (which may well be nothing), I think changes such as replacing:

'+brwiki' => [
	'Discussion_Wikipedia' => NS_PROJECT_TALK,
],

with:

'+brwiki' => [ 'Discussion_Wikipedia' => NS_PROJECT_TALK, ],

actually make the file harder to use, since the latter formatting would have to be changed back to the former to add more entries/settings for the same wiki. (In this case, this is under 'wgNamespaceAliases', but the same goes for 'wgExtraNamespaces' and other places in the file.)
This also adversely affects readability (IMO), since most of the wikis do have multiple entries/settings (sorry, I don't know the proper terminology) for those variables, and only a few have a single one.
I would recommend not making this kind of change unless the vast majority (if not all) of the entries can use the single-line format (such the changes made to 'wgProofreadPageNamespaceIds').

Sure, that makes sense. My rule of thumb is that if an array has 1 or 2 entries, it can be on 1 line, and if it has more than 3, it should be split (if it has 3, ignore it) but I can see where that wouldn't be everyone's views. What about in the group overrides, where many entries are 1 line?

Dcljr added a comment.Sep 2 2019, 9:57 AM

What about in the group overrides, where many entries are 1 line?

Uhh… I don't know. [grin]

Obviously, the views of those who work on the file regularly are more salient, but my idea is if the settings in a variable are "relatively likely" to be added to in the future (as with NSs and NS aliases), the entries should probably be kept in a multiple-line format to aid with those later additions (similarly to how the final entry in an array typically ends with a comma even though it doesn't need to).

Urbanecm triaged this task as Lowest priority.Sep 3 2019, 9:31 AM

The discussion is worth future, I guess. I wonder what to do if I want add an entry to that currently oneline definitions. To solve that, I would like to propose "preferred is multiline, oneline is permitted for entries that are highly unlikely to change". That way, we'll deal with possible future aditions easily, but autopatrolled groups would be more readable, I guess.

Dcljr added a comment.Sep 17 2019, 3:59 AM

Uh… so now there are no per-wiki settings in InitialiseSettings.php at all! What is going on? (T233069)

Apparently planned cleanup,, see T223602.

Change 544001 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[operations/mediawiki-config@master] Partial cleanup of InitialiseSettings

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

@Urbanecm can you take a look at the latest patch? Its smaller in scope, and should be easier to review

Change 544001 merged by jenkins-bot:
[operations/mediawiki-config@master] Partial cleanup of InitialiseSettings

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

Mentioned in SAL (#wikimedia-operations) [2019-10-21T12:01:55Z] <urbanecm@deploy1001> Synchronized wmf-config/InitialiseSettings.php: SWAT: e8d70c1: Partial cleanup of InitialiseSettings (T231178) (duration: 01m 00s)

Change 544982 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[operations/mediawiki-config@master] Partial cleanup of InitialiseSettings

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

Change 544982 abandoned by DannyS712:
Partial cleanup of InitialiseSettings

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

Change 546369 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[operations/mediawiki-config@master] Partial cleanup of InitializeSettings

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

@Urbanecm can you take a look? Its smaller in scope and should be easier to review - this patch is mostly just formatting.

Change 546369 merged by jenkins-bot:
[operations/mediawiki-config@master] Partial cleanup of InitializeSettings

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

Mentioned in SAL (#wikimedia-operations) [2019-11-20T11:46:15Z] <urbanecm@deploy1001> Synchronized wmf-config/InitialiseSettings.php: SWAT: 51ecd71: Partial cleanup of InitializeSettings (T231178) (duration: 00m 52s)