r81087 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r81086‎ | r81087 | r81088 >
Date:10:57, 27 January 2011
Author:jeroendedauw
Status:reverted (Comments)
Tags:
Comment:
Added a warning so non-mediawiki-developers understand that they are getting a snapshot from trunk and not something that has been tested to be compatible with the selected MW version or work at all
Modified paths:
  • /trunk/extensions/ExtensionDistributor/ExtensionDistributor.i18n.php (modified) (history)
  • /trunk/extensions/ExtensionDistributor/ExtensionDistributor_body.php (modified) (history)

Diff [purge]

Index: trunk/extensions/ExtensionDistributor/ExtensionDistributor_body.php
——@@ -199,12 +199,16 @@
200200 Xml::submitButton( wfMsg( 'extdist-submit-version' ), array( 'name' => 'extdist_submit' ) ) .
201201 Xml::closeElement( 'form' ) . "\n"
202202 );
 203+
 204+ $wgOut->addHtml( '<span style="color:darkred">' . wfMsgExt( 'extdist-trunk-warning', 'parse' ) . '</span>' );
203205 }
204206
205207 protected function doDownload( $extension, $version ) {
206208 global $wgExtDistWorkingCopy, $wgExtDistTarDir, $wgExtDistBranches,
207209 $wgOut, $wgExtDistTarUrl, $wgExtDistRemoteClient;
208210
 211+ $wgOut->addHtml( '<span style="color:darkred">' . wfMsgExt( 'extdist-trunk-warning', 'parse' ) . '</span>' );
 212+
209213 if ( $wgExtDistRemoteClient ) {
210214 $rev = $this->updateAndGetRevisionRemote( $extension, $version );
211215 } else {
Index: trunk/extensions/ExtensionDistributor/ExtensionDistributor.i18n.php
——@@ -47,6 +47,7 @@
4848
4949 If you have any questions about this extension distribution system, please go to [[Extension talk:ExtensionDistributor]].",
5050 'extdist-want-more' => 'Get another extension',
 51+ 'extdist-trunk-warning' => "'''Warning''': this tool will get you a development snapshot that might very well '''not work''' with the MediaWiki version you selected or be broken altogether and is likely to be '''out of date'''.",
5152 );
5253
5354 /** Message documentation (Message documentation)

Follow-up revisions

RevisionCommit summaryAuthorDate
r93896Revert r81087, consensus was never reached, and the discussion not finished...reedy16:26, 4 August 2011

Comments

#Comment by 😂 (talk | contribs)   13:39, 27 January 2011

How would getting a snapshot from trunk be out of date? The wording doesn't make sense here.

#Comment by Jeroen De Dauw (talk | contribs)   13:55, 27 January 2011

You are getting a snapshot from code that was branched from trunk and then never updated, so of course it'll be outdated if any development on it is done.

#Comment by 😂 (talk | contribs)   14:04, 27 January 2011

Well of course if you download it it won't be updated.

The way you've phrased it, it sounds like "This download you're grabbing right now might already be out of date" which is just not true when referring to trunk.

#Comment by Jeroen De Dauw (talk | contribs)   14:21, 27 January 2011

The warning is not for trunk, it's for everything else. Right now people are using the extension distributor, selecting the MediaWiki release they are using, such as 1.16, and are then getting a snapshot of something that was branched from trunk together with MW 1.16. You and I might know this, but most people using it are assuming they are getting an up to date version for 1.16. And are assuming that since different MW versions are listed, the one they are getting is compatible with the version they selected. I've seen many people make this mistake, so a warning will obviously help prevent confusion.

#Comment by 😂 (talk | contribs)   14:24, 27 January 2011

The message key (and commit message) implied you're warning about trunk :)

#Comment by Jeroen De Dauw (talk | contribs)   14:37, 27 January 2011

It is coming from trunk...

  • The code is on trunk
  • The code is branched
  • The snapshot is made

-> you end up getting code just pulled from trunk at a pretty random point for most extensions

I am not saying the code is coming from the current trunk.

#Comment by 😂 (talk | contribs)   14:50, 27 January 2011

Thanks for explaining the branching process!

I'm not trying to say you're wrong. I'm saying your original commit message (and the message key) caused me to misunderstand your warning.

And I think the warning is a little too stern. We're pretty damn sure that REL1_16 ParserFunctions is going to work with a 1.16 wiki. You're saying "Warning" and bolding "not work" and "out of date" with quite an emphasis.

Scenario: I'm a wiki admin, running 1.16.x. I want to download some extensions. Well I certainly don't want the development copy running on my production server! But oh shit, the 1.16 version is warning me about compatibility issues.

I know our branch-with-core system sucks (well, it sucks for any extension trying to handle their own releases), but it does work most of the time.

#Comment by Jeroen De Dauw (talk | contribs)   15:01, 27 January 2011

ParserFunctions sure, but pretty much all other extensions... You can put in a check against a hardcoded array of "always stable" extensions of course.

#Comment by 😂 (talk | contribs)   15:20, 27 January 2011

Hardcoded is even worse.

I still haven't seen any reason why a REL1_16-branched extension shouldn't work with 1.16.x.

Of course it won't contain the latest bug fixes and features, but it certainly shouldn't be broken. That implies it was broken at the branch point (ie: already broken in trunk), or something was backported that broke it (I can't see this happening easily/mistakingly).

#Comment by Jeroen De Dauw (talk | contribs)   15:26, 27 January 2011

Do you think this change should be reverted? Do you thing it should be modified further?

If so, go ahead.

Not having the warning leads to people installing outdated code in many cases. I've seen this time and time again, so if you think warning people about it is wrong, then we're not going to agree.

#Comment by 😂 (talk | contribs)   15:33, 27 January 2011

(unindenting)

I'm not sure. I see what you mean by the warning, I'm just not sure what the best way to put it is. Maybe something like this:

"You are downloading an extension branched with MediaWiki XX. While this will work with MediaWiki XX, it might not have the latest bug fixes and features in newer branches or in the development copy"

But along with the development copy:

"This is the latest development copy of Extension:FooBar. It is only guaranteed to work with the latest development version of MediaWiki, and may not work with older version."

My wording sucks because I'm an engineer and not a writer, but I think having both of these warnings in tandem would work. My main concern in the current wording is off-putting people from using an extension because they think it's old when in fact, it was just branched like a month ago, as an example.

#Comment by Jeroen De Dauw (talk | contribs)   15:47, 27 January 2011

"While this will work with MediaWiki XX" and "It is only guaranteed to work with the latest development version of MediaWiki" are simply not true; there is no guarantee anywehere. Feel free to reword the message, as long as it makes it absolutely clear that there is no guarantee whatsoever the extension works or is compatible. The misconception most people have is that the thing they get was manually specified for that version of MediaWiki. What you wrote just encourages this misconception.

#Comment by 😂 (talk | contribs)   15:49, 27 January 2011

We're going in circles...like I said above, why *wouldn't* a REL1_16 extension work with 1.16.x?

#Comment by Reedy (talk | contribs)   17:43, 27 June 2011

Can we decide what to do here, whether revert, fix the message or whatever to finish this off?

#Comment by 😂 (talk | contribs)   17:50, 27 June 2011

I still say revert, but I'm not willing to argue this in circles again.

Status & tagging log