Remove methods which have been deprecated before 1.0

[Benestar]

These methods have been deprecated before our first major release and there are simple replacements for all of them (when #474 is merged). Therefore, those methods can be removed in the next major deploy (3.0 perhaps).

This depends on #475and #474

Tracked in https://phabricator.wikimedia.org/T98290

[thiemowmde]

-2. As long as https://phabricator.wikimedia.org/T87237 is not solved no new instances of Fingerprint should be introduced.

[Benestar]

Really?? An issue without an update for 2 months is a blocker for such patches? This is quite frustrating you must know... 😞

[thiemowmde]

We did not decided to work on this at the moment. This causes a hell lot of breaking changes in all repositories. This will bind an enormous amount of developers time, which is just not necessary. Not at this point in time. Maybe for 4.0, but not now. Not touching these convenience methods does not cause any major trouble that must be fixed within the next days or weeks. It was like that for years, it can easily stay like that for some more weeks, until the discussion was picked for a sprint and resolved.

So -2 for now.

Also see #427.

[Benestar]

The pure amount of breaking changes might be huge, but it is really simple to resolve them (see how it is done in the tests, simple text replacement). If that makes you think different, I can also prepare a patch for Wikibase which removes usages of this deprecated methods, but perhaps you will -2 there as well because that would introduce even more usages of Fingerprint.

"within the next days or weeks" → this is rather the next months given the activity on the related bug. My intention is to get the refactoring of Entity done so that we can support more Entity types (needed for Structured Wikiquote for example). However, if this is prioised so low from the side of the dev team I will have to wait with those changes until you'll get to that feature I suppose. Are there any plans when exactly you want to start tackling the issues related to Entity?

[thiemowmde]

I would oppose such a commit on Wikibase.git because it makes the code ugly and harder to read and maintain.

I think this patch is approaching the problem from the wrong side. Simply killing methods does not make the situation better. All it does is forcing all other developers to spend time on something that is not on any board at the moment. We know we want to work on that, but we don't know when.

More discussion at https://phabricator.wikimedia.org/T98290#1264595.

[Benestar]

Actually it does not make the code ugly but makes it less dependent on Entity related things. Replacing this methods by Fingerprint or EntityTerms or whatever is a good thing as we want to move away from Entity. Removing methods form Entity means that Entity will be used in fewer places.

[thiemowmde]

As I said, approaching from the wrong side. This patch does not reduce the number of usages of Entity. -2.

[Benestar]

Please add comments on the dedicated task on phabricator, not here on github, so that more people are aware of the ongoing discussion.

[thiemowmde]

I already said everything on Phabricator I have to say. All this patch does is hurting us. Nothing is blocked on the removal of deprecated methods. -2.

[Benestar]

So why do you keep saying the same thing over and over again in this PR as well? Repeating -2 several times does not make them a -6.

[JeroenDeDauw]

I think it is premature to try to remove these methods from Entity. At this point there are still many places where one has something of type Entity on which these methods are invoked. Yes, such code is very likely problematic and ought to change. Forcing all such issues to be fixed to use a new DM release will make the usage extremely hard.

[Benestar]

My intention was that it would be easier to fix code dealing with Fingerprint objects instead of directly calling methods of Entity so we can easily replace Entity with FingerprintProvider at some point.

However, as this major release is focused on factoring out Claims I can also understand if you delay this patch for a later release. This is something we should discuss though. The next major release might not be tomorrow.

I will try to find out how much refactoring costs this change introduces in Wikibase Repository and other components so we have some actual data we can rely on.

[JeroenDeDauw]

My intention was that it would be easier to fix code dealing with Fingerprint objects instead of directly calling methods of Entity so we can easily replace Entity with FingerprintProvider at some point.

Easier how? Politically? I'm not seeing how this makes any technical task easier.

[Benestar]

Easier in the sense of first using the Fingerprint methods and then switching to FingerprintProvider. I thought splitting these two parts would be a good idea but perhaps that is a bad assumption from my side, so we should perhaps do this sometimes later.

[JeroenDeDauw]

I'm not sure we are talking about the same thing. My objections are against the "Remove methods which have been deprecated before 1.0" commit. I think "Replace usage of deprecated methods with alternatives" is fine.

[thiemowmde]

@Benestar, can we close this in favor of multiple much smaller patches that can be merged later one by one?

[Benestar]

@thiemowmde sure :-)