MediaWiki r88025 - Code Review

Jump to: navigation, search
Repository:MediaWiki
Revision:r88024‎ | r88025 (on ViewVC)‎ | r88026 >
Date:23:31, 13 May 2011
Author:freakolowsky
Status:reverted (Comments)
Tags:
Comment:
* removed unused messages because of previous revert.
* hidden namespace select box if in wgMiserMode(requested by domas)
Modified paths:

Diff [purge]

Loading diff…

Follow-up revisions

Rev.Commit summaryAuthorDate
r88026* addon to previousfreakolowsky23:42, 13 May 2011
r96306Revert r88008 (add size difference to Special:Contributions) and its large gr...catrope21:47, 5 September 2011
r99102Revert r88025 (put Special:Contributions namespace filter behind $wgMiserMode...catrope13:44, 6 October 2011

Comments

#Comment by Raymond (talk | contribs)   09:27, 14 May 2011

This is a really great regression for authors's usability. Please add an index before this revision goes live on WMF

#Comment by Catrope (talk | contribs)   09:31, 14 May 2011

This isn't really indexable.

#Comment by Church of emacs (talk | contribs)   09:43, 22 July 2011

A bad regression indeed. Is there no way this could be implemented in another way? Is the current code too inefficient even for small wikis?

#Comment by Hashar (talk | contribs)   16:34, 18 May 2011

Marking OK. The code is fine, it is better to remove the NS filtering than making the whole project slow.

#Comment by Duplicatebug (talk | contribs)   16:52, 25 May 2011

You can allow namespace filter, when the recentchanges table is in the query, than only the last few days are filterable for namespaces, but better than nothing. That filter is already on Special:RecentChanges, so there should be no problem. Thanks.

#Comment by MaxSem (talk | contribs)   09:49, 22 July 2011

Eh, so miser mode is enforced in UI, but it still can be overridden with a URL parameter?

#Comment by Krinkle (talk | contribs)   22:03, 5 October 2011

Fixed in r88026

#Comment by Foroa (talk | contribs)   15:31, 5 October 2011

The system is slower and the "contributions page filtered for namespace or RevisionDeleted edits" removal is a drama. I use that hundred times per day.

#Comment by Raymond (talk | contribs)   16:31, 5 October 2011

+1 Unable to filter user contributions by namespace is really painfully.

#Comment by Bulwersator (talk | contribs)   18:30, 5 October 2011

It is terrible change. Maybe it may be better to limit checking only 100 edits of user with default settings and option to fetch more?

#Comment by Catrope (talk | contribs)   19:04, 5 October 2011

There is no way to express that in SQL. Could be done on the client side, though.

#Comment by Catrope (talk | contribs)   19:04, 5 October 2011

Ahm and by 'client side' I meant PHP :)

#Comment by Chaddy (talk | contribs)   21:58, 5 October 2011

Wtf? Please undo this "improvement" immediately... -_-

#Comment by Krinkle (talk | contribs)   22:04, 5 October 2011

Please read the given comments and discussion. The query to fetch contributions by namespace is not indexed on misermode-wikis, thus this query will not be allowed on those wikis.

#Comment by Crossmr (talk | contribs)   23:21, 5 October 2011

That's not good enough. This was a feature in use by countless people and extremely helpful.

#Comment by Reedy (talk | contribs)   23:25, 5 October 2011
#Comment by Chaddy (talk | contribs)   23:40, 5 October 2011

? Sorry, I don't understand anything of your technical terms...

This function worked for many years quite fine thus I don't see any reason to take away this feature from us (what's actually the problem? too much performance needed by these filters?). Most of the Wikipedians need this function strongly. What about first thinking about the consequences before just tinkering with the software? Introducing senseless and precarious image filters is obviously no problem but a widely used function that worked fine for many years is? Hmm...

(Please excuse my strong language but this is the icing on the cake of an already arduous day...)

#Comment by Happy-melon (talk | contribs)   23:48, 5 October 2011

If you're already having a bad day I won't add to it with an aggressive reply, but suffice to say that you're correct in your estimation: implementing this filter requires too many server resources. Domas, who apparently requested this change, is our most experienced database 'guru'; if he says something is wrong, it's usually because it is.

#Comment by Ca$e (talk | contribs)   07:34, 6 October 2011

that's *not nearly* good enough. there would have been many ways to limit db resource occupation without subverting usability to *that* degree. just taken from bugzilla 31197, e.g.: A)

look over last n edits
kill query after 10 seconds
cache results

B)

disable for users where
subject user_editcount is 100,000 or more, and 
invokers don't have a new permission like noquerylimit (similar to noratelimit)
which would be given to administrators and maybe rollbackers

and there would be lots more.

  • do* also consider lots of already posted arguments countering your replies, e.g.
Users need this (finding edits in specific namespaces), therefore if you don't allow this feature they'll just load the 
complete list of edits, 5000 at a time, and search the namespace within the pages. In your example: 
Rambot, 140 000 edits, 28 pages; for the first I got "Served by mw30 in 14.588 secs", this makes a total of almost 7 minutes."

i strongly oppose the way such "fixes" are brought up without prior discussion with the people actually maintaining wp and expect a quick solution!

#Comment by Optimist on the run (talk | contribs)   12:01, 6 October 2011

Definitely not a good move - please restore the functionality, or revert back to previous version (which would solve all the other bugs that seem to have been introduced - see WP:VPT on enwiki).

Status & tagging log

  • 13:45, 6 October 2011 Catrope (talk | contribs) changed the status of r88025 [removed: ok added: reverted]
  • 16:34, 18 May 2011 Hashar (talk | contribs) changed the status of r88025 [removed: fixme added: ok]
  • 09:27, 14 May 2011 Raymond (talk | contribs) changed the status of r88025 [removed: new added: fixme]