Page MenuHomePhabricator

Tidy up wmf-config CommonSettings.php and InitialiseSettings.php
Open, LowPublic

Description

These files seem to have a mix of indenting (though, more usually spaces)

They also still have numerous bits of code which are ancient/commented out

Where we're doing lots of different assignments to a global variable, per lang/project, some aren't in alphabetical order, with some of them grouped by project

All things that make the files more unreadable, and difficult to follow


Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
bzimport added a subscriber: Unknown Object (MLST).

Is there a reason no one does this? I'd like to clear some crap.

(In reply to comment #1)

Is there a reason no one does this? I'd like to clear some crap.

Fear of breaking the site?

(In reply to comment #2)

(In reply to comment #1)

Is there a reason no one does this? I'd like to clear some crap.

Fear of breaking the site?

A lot of this stuff was *commented out* for ages.

As one of the bastards who edited that file in crappy ssh terminals over several years, I'M SOOOO SOOORRRRRYYYYYYYYYYYY

(In reply to comment #4)

As one of the bastards who edited that file in crappy ssh terminals over
several years, I'M SOOOO SOOORRRRRYYYYYYYYYYYY

Well, at least you're sorry ;D

(In reply to comment #3)

(In reply to comment #2)

(In reply to comment #1)

Is there a reason no one does this? I'd like to clear some crap.

Fear of breaking the site?

A lot of this stuff was *commented out* for ages.

Exactly. I'm sure it's a mixture of things. It'd just be nice to tidy this up for future usage. No doubt it may regress again, but at least it'll be in better shape for a while...

I see sorta 4 steps

  1. Clear anything commented that is obviously ancient
  2. Clear things that aren't commented that are now old/changed already by core etc
  3. Fix whitespace
  4. Rearrange stuff to correct ordering (more stuff under globals, rather than at the global level)

reedy@fenari:/home/wikipedia/common/php-1.17/wmf-config$ wc -l InitialiseSettings.php
9533 InitialiseSettings.php
reedy@fenari:/home/wikipedia/common/php-1.17/wmf-config$ wc -l CommonSettings.php
2487 CommonSettings.php

Also

Fixing any usages of Magic numbers, especially for namespaces where NS_PROJECT etc constants exist

Something for 1.18, is remembering to remove

Load site configuration

include( "$IP/includes/DefaultSettings.php" );

As Tim moved it out of LocalSettings for 1.18 IIRC

Now that these files are in Git, anyone can start working on this, right?

(In reply to comment #5)

  1. Clear anything commented that is obviously ancient
  2. Clear things that aren't commented that are now old/changed already by core

etc

  1. Fix whitespace
  2. Rearrange stuff to correct ordering (more stuff under globals, rather than

at the global level)

I'd move step 3 to step 1, but otherwise this seems sane. This definitely should be done in steps, as subtle changes (particularly rearranging code) can have weird effects.

I think step 1 here is to fix up the whitespace and indentation mess. Once that gets merged and deployed, the future editing will be easier and the future diffs will be readable.

(In reply to comment #9)

Now that these files are in Git, anyone can start working on this, right?

Yes.

Its probably best to do these steps in separate easy-to-review commits, to makes review and deploy can happen fast without feating for rebasing such mess.

Taking this bug.


Removed old comments lines.
CommonSettings.php doesn't seem to contain old comments

Gerrit change 23059


I did Reedy's step 1 first, as it mainly consist to remove lines and fix spaces to lines to delete immediately after didn't make sense.

I agree next step should be the whitespaces trimming (and I'm volunteer to do it as soon as the current step 1 configuration change is okay).


Would someone know how to contact proteins MSU lab? I wanted to check with the lab first if they still need their rate limit extemption, but got a mail delivery error - the laboratory pages don't provide more contact information nor sign the project is still active.

Related URL: https://gerrit.wikimedia.org/r/65860 (Gerrit Change I2700c3cfc0a4f43077a0df78d65235f1ac48bffa)

Change 78637 had a related patch set uploaded by TTO:
Continuing to clean up InitialiseSettings.php

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

Change 78637 merged by jenkins-bot:
Continuing to clean up InitialiseSettings.php

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

Change 95284 had a related patch set uploaded by Odder:
(bug 29902) Clean up InitialiseSettings.php, step 4/∞

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

Change 65860 abandoned by Hashar:
(bug 29902) Tidied up CommonSettings and InitialiseSettings

Reason:
Most of the whitespaces / bug # formatting seems to have been corrected already. Thus abandoning this very old change.

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

Change 95284 merged by jenkins-bot:
(bug 29902) Clean up InitialiseSettings.php, step 4/∞

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

All patches mentioned in this report are either merged or abandoned - is there more work left to do here (if yes: please reset the bug report status to NEW or ASSIGNED), or can you close this ticket as RESOLVED FIXED?

There's still work to do, e.g. wgMetaNamespace(Talk)? and wgNamespacesWithSubpages, and minor indent cleanup as well. Reordering stuff in the file is just annoying, as it gets in the way of git blame.

It seems that the days of InitialiseSettings as we know it are numbered, with the new Configuration work being undertaken at the moment. So in my personal opinion it would not be unreasonable to close this bug once those basic tasks are done.

Change 154455 had a related patch set uploaded by TTO:
Clean up indents, comments, spacing in InitialiseSettings

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

Change 156078 had a related patch set uploaded by Withoutaname:
Remove unused variables and commented-out code from CommonSettings

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

Glaisher lowered the priority of this task from Medium to Low.Mar 10 2015, 4:24 PM
Glaisher set Security to None.
Glaisher updated the task description. (Show Details)
Glaisher removed a subscriber: Unknown Object (MLST).

This is not General/Unknown. Configuration matters traditionally go in Site requests unless there is a more specific component; there is #Wikimedia-Deployment-systems but that might be considered a bit of a stretch.

Continuing the OT, the Site-requests project is, to me, ill-defined. Traditionally, stuff to do with *Settings.php has gone in there, but that doesn't have to be the case anymore in Phabricator. See T90468 for some notes I wrote down.

I removed it from #site-requests as it doesn't actually change a configuration on wiki. I don't mind much about which project this is in though.

Change 199574 had a related patch set uploaded (by Alex Monk):
Make references to tasks/bugs more consistent

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

Change 199574 merged by jenkins-bot:
Make references to tasks/bugs more consistent

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

Change 199803 had a related patch set uploaded (by Alex Monk):
Remove another useless wgMasterWaitTimeout reference

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

Change 199803 merged by jenkins-bot:
Remove another useless wgMasterWaitTimeout reference

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

What else do we want to do here?

What else do we want to do here?

Standardize and fact-check the comments, I think. There's an ugly mix of // T123 and # bug T123, in addition to a number of questionable comments.

Change 221803 had a related patch set uploaded (by Alex Monk):
Standardise a ton of ticket comments

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

Change 221808 had a related patch set uploaded (by Alex Monk):
Remove wmgUseXAnalytics and wgAjaxEditStash override, other random cleanup

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

Change 221809 had a related patch set uploaded (by Alex Monk):
Standardise remaining ticket comments I could find

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

Change 221803 merged by jenkins-bot:
Standardise a ton of ticket comments

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

Change 221808 merged by jenkins-bot:
Remove wmgUseXAnalytics and wgAjaxEditStash override, other random cleanup

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

Change 221809 merged by jenkins-bot:
Standardise remaining ticket comments I could find

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

What else do we want to do here?

Standardize and fact-check the comments, I think. There's an ugly mix of // T123 and # bug T123, in addition to a number of questionable comments.

What about now? What other questionable comments did you find?

Turning this into a tracker, specific tasks can be added as blockers

Krenair renamed this task from Tidy up wmf-config CommonSettings.php and InitialiseSettings.php to Tidy up wmf-config CommonSettings.php and InitialiseSettings.php (tracking).Aug 12 2015, 1:34 PM
Krenair moved this task from Backlog to Tracking on the Wikimedia-Site-requests board.

What about now? What other questionable comments did you find?

There are some //T vs. // T.

Change 236701 had a related patch set uploaded (by Alex Monk):
Tidy up more comments

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

Change 236701 merged by jenkins-bot:
Tidy up more comments

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

Phabricator_maintenance renamed this task from Tidy up wmf-config CommonSettings.php and InitialiseSettings.php (tracking) to Tidy up wmf-config CommonSettings.php and InitialiseSettings.php.Aug 13 2016, 10:11 PM