Last modified: 2014-11-07 19:16:25 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T31902, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 29902 - Tidy up wmf-config CommonSettings.php and InitialiseSettings.php
Tidy up wmf-config CommonSettings.php and InitialiseSettings.php
Status: PATCH_TO_REVIEW
Product: Wikimedia
Classification: Unclassified
Site requests (Other open bugs)
unspecified
All All
: Normal enhancement with 1 vote (vote)
: ---
Assigned To: Nobody - You can work on this!
https://gerrit.wikimedia.org/r/gitweb...
: easy
Depends on: 40075 53790 53909
Blocks:
  Show dependency treegraph
 
Reported: 2011-07-14 23:32 UTC by Sam Reed (reedy)
Modified: 2014-11-07 19:16 UTC (History)
16 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Sam Reed (reedy) 2011-07-14 23:32:38 UTC
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
Comment 1 Aaron Schulz 2011-07-15 00:10:47 UTC
Is there a reason no one does this? I'd like to clear some crap.
Comment 2 p858snake 2011-07-15 00:16:41 UTC
(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?
Comment 3 Aaron Schulz 2011-07-15 00:17:16 UTC
(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.
Comment 4 Brion Vibber 2011-07-15 00:26:00 UTC
As one of the bastards who edited that file in crappy ssh terminals over several years, I'M SOOOO SOOORRRRRYYYYYYYYYYYY
Comment 5 Sam Reed (reedy) 2011-07-15 10:47:25 UTC
(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)
Comment 6 Sam Reed (reedy) 2011-07-15 11:23:57 UTC
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
Comment 7 Sam Reed (reedy) 2011-07-17 19:50:08 UTC
Also

Fixing any usages of Magic numbers, especially for namespaces where NS_PROJECT etc constants exist
Comment 8 Sam Reed (reedy) 2011-08-01 16:49:17 UTC
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
Comment 9 MZMcBride 2012-08-09 18:41:44 UTC
Now that these files are in Git, anyone can start working on this, right?
Comment 10 MZMcBride 2012-09-02 19:05:20 UTC
(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
> 3) Fix whitespace
> 4) 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.
Comment 11 Krinkle 2012-09-02 19:13:17 UTC
(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.
Comment 12 Dereckson 2012-09-07 12:14:54 UTC
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.
Comment 13 Gerrit Notification Bot 2013-05-29 01:08:58 UTC
Related URL: https://gerrit.wikimedia.org/r/65860 (Gerrit Change I2700c3cfc0a4f43077a0df78d65235f1ac48bffa)
Comment 14 MZMcBride 2013-08-11 14:29:02 UTC
https://gerrit.wikimedia.org/r/78227 has been merged.
Comment 15 Gerrit Notification Bot 2013-08-12 07:16:20 UTC
Change 78637 had a related patch set uploaded by TTO:
Continuing to clean up InitialiseSettings.php

https://gerrit.wikimedia.org/r/78637
Comment 16 Gerrit Notification Bot 2013-09-05 18:22:27 UTC
Change 78637 merged by jenkins-bot:
Continuing to clean up InitialiseSettings.php

https://gerrit.wikimedia.org/r/78637
Comment 17 Gerrit Notification Bot 2013-11-13 22:05:18 UTC
Change 95284 had a related patch set uploaded by Odder:
(bug 29902) Clean up InitialiseSettings.php, step 4/∞

https://gerrit.wikimedia.org/r/95284
Comment 18 Gerrit Notification Bot 2013-12-09 13:37:22 UTC
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
Comment 19 Gerrit Notification Bot 2013-12-19 19:10:22 UTC
Change 95284 merged by jenkins-bot:
(bug 29902) Clean up InitialiseSettings.php, step 4/∞

https://gerrit.wikimedia.org/r/95284
Comment 20 Andre Klapper 2014-08-17 10:59:27 UTC
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?
Comment 21 This, that and the other (TTO) 2014-08-17 11:09:13 UTC
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.
Comment 22 Gerrit Notification Bot 2014-08-17 11:30:03 UTC
Change 154455 had a related patch set uploaded by TTO:
Clean up indents, comments, spacing in InitialiseSettings

https://gerrit.wikimedia.org/r/154455
Comment 23 Gerrit Notification Bot 2014-08-25 06:31:33 UTC
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

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links