Last modified: 2013-03-13 19:05:08 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 T35287, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 33287 - Get rid of svn keywords in API files
Get rid of svn keywords in API files
Status: REOPENED
Product: MediaWiki
Classification: Unclassified
API (Other open bugs)
unspecified
All All
: Normal normal with 1 vote (vote)
: ---
Assigned To: Nobody - You can work on this!
:
: 38301 (view as bug list)
Depends on:
Blocks: 35885
  Show dependency treegraph
 
Reported: 2011-12-21 00:33 UTC by Chad H.
Modified: 2013-03-13 19:05 UTC (History)
12 users (show)

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


Attachments

Description Chad H. 2011-12-21 00:33:43 UTC
They're going to be useless in Git. Need to come up with a new versioning scheme.
Comment 1 Sam Reed (reedy) 2011-12-21 01:04:18 UTC
(In reply to comment #0)
> They're going to be useless in Git. Need to come up with a new versioning
> scheme.

Or do we really need to keep them at all?

They have some use, but I can't really say I've ever used [1] for anything specifically, and similarly for [2]

[1] http://en.wikipedia.org/w/api.php?version
[2] http://en.wikipedia.org/w/api.php?action=paraminfo&modules=parse
Comment 2 Chad H. 2011-12-21 01:44:18 UTC
It would be nice to have some kind of versioning on our public API so people can detect incompatibilities when they exist.
Comment 3 Antoine "hashar" Musso (WMF) 2012-02-17 08:48:32 UTC
We could do that using a global array just like we did to track stylesheet version. Or define a constant for each API class much like Parser::VERSION or Parser_LinkHooks::VERSION.
Comment 4 Chad H. 2012-02-17 12:56:50 UTC
(In reply to comment #3)
> We could do that using a global array just like we did to track stylesheet
> version. Or define a constant for each API class much like Parser::VERSION or
> Parser_LinkHooks::VERSION.

I'd much prefer the latter. As long as we can define a clear policy for how often and when the version numbers should be bumped.
Comment 5 Krinkle 2012-04-14 04:18:26 UTC
(In reply to comment #2)
> It would be nice to have some kind of versioning on our public API so people
> can detect incompatibilities when they exist.

Although the previous svn:keywords did not provide that information, that was just a simple "last rev modified" number. Right now this bug is just to remove them. We could open a feature enhancement ticket for that other, although I personally never found that needed and release notes seem to be more useful for breaking changes. If we create an API module to programmatically be able to export the version of an Api module, the it could be useful to an Api client to see if it considers itself compatible, that'd be a nice feature.
Comment 6 Antoine "hashar" Musso (WMF) 2012-04-16 14:26:41 UTC
git has the ability to fill in keywords when checking out files. Have a look at "Keyword Expansion" on http://progit.org/book/ch7-2.html .
Comment 7 Max Semenik 2012-04-16 14:30:51 UTC
Solutions that require local configuration changes will not work as nobody will use them.
Comment 8 Chad H. 2012-04-16 14:52:00 UTC
(In reply to comment #6)
> git has the ability to fill in keywords when checking out files. Have a look at
> "Keyword Expansion" on http://progit.org/book/ch7-2.html .

Just because the functionality exists doesn't mean we should use it. There's lots of bad ideas out there ;-)

(In reply to comment #7)
> Solutions that require local configuration changes will not work as nobody will
> use them.

This.

(In reply to comment #5)
> (In reply to comment #2)
> > It would be nice to have some kind of versioning on our public API so people
> > can detect incompatibilities when they exist.
> 
> Although the previous svn:keywords did not provide that information, that was
> just a simple "last rev modified" number. Right now this bug is just to remove
> them. We could open a feature enhancement ticket for that other, although I
> personally never found that needed and release notes seem to be more useful for
> breaking changes. If we create an API module to programmatically be able to
> export the version of an Api module, the it could be useful to an Api client to
> see if it considers itself compatible, that'd be a nice feature.

I'm in favor of simple integer increments. When a module changes in an incompatible way, bump v1 -> v2 (which doesn't happen *often*). But you're right, let's spin that off to another bug and just remove the existing keywords while we bikeshed over that.
Comment 9 Iaroslav Vassiliev 2012-05-02 20:36:01 UTC
Some kind of versioning is definitely required.
Comment 10 Niklas Laxström 2012-06-20 10:23:44 UTC
Are you going to do something about this?
Comment 11 Antoine "hashar" Musso (WMF) 2012-06-20 11:49:06 UTC
Gerrit change https://gerrit.wikimedia.org/r/#/c/12155/
Comment 12 Chad H. 2012-06-20 11:55:26 UTC
(In reply to comment #11)
> Gerrit change https://gerrit.wikimedia.org/r/#/c/12155/

There are dozens of core API files that need changing too. Plus all other extensions using this.

Like I suggested before, I think we should change this to return an integer, and increment it by 1 anytime a module changes in an incompatible way.
Comment 13 Antoine "hashar" Musso (WMF) 2012-06-20 13:18:55 UTC
Go ahead Chad. I will be glad to +2 / merges such a change.
Comment 14 Antoine "hashar" Musso (WMF) 2012-07-11 09:38:56 UTC
*** Bug 38301 has been marked as a duplicate of this bug. ***
Comment 15 Krinkle 2012-07-12 13:55:05 UTC
Lets start by making the abstract method implemented, so new Api modules people create don't need this method. Just ran into this during a tutorial.

"Mr. Krinkle, what's the $Id:$ line for?" :-)
Comment 16 Sam Reed (reedy) 2012-07-12 15:46:31 UTC
(In reply to comment #15)
> Lets start by making the abstract method implemented, so new Api modules people
> create don't need this method. Just ran into this during a tutorial.
> 
> "Mr. Krinkle, what's the $Id:$ line for?" :-)

Need to make it return something... $wgVersion

Oh, and remove "Usually done with SVN's Id keyword" at the same time xD
Comment 17 Yuri Astrakhan 2013-03-04 05:31:56 UTC
Now that SVN Id keywords has been removed by I910ca1448ed2ed697ac19b17c486d130aa1d7e03 - should I close this bug too?
Comment 18 Antoine "hashar" Musso (WMF) 2013-03-08 22:46:57 UTC
I guess :-]
Comment 19 Niklas Laxström 2013-03-09 14:06:42 UTC
These are still present in many extensions and in http://www.mediawiki.org/wiki/API:Extensions

At least the documentation must be updated to reflect the current practice.
Comment 20 Yuri Astrakhan 2013-03-13 19:05:08 UTC
Updated docs. The extensions might need to be updated later, as they sometimes get deployed before the new core.

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


Navigation
Links