search

Pryaxis / TShock

☕️⚡️TShock provides Terraria servers with server-side characters, anti-cheat, and community management tools.
GNU General Public License v3.0
2.41k stars 377 forks source link

How do we handle deprecated methods. #883

Closed Olink closed 9 years ago

Olink commented 9 years ago

This is a topic of debate. Currently a deprecated method can be removed any time after a release that includes the deprecation. However, there is no requirement of this deprecation removal being made public to people who may be using the deprecated methods. While it is the developers fault for not removing it, it is also our fault for not giving them warning. When the precedence is never removing deprecated methods I believe that there should be some discussion if a one week warning (the same as API changes) should be given.

I vote yes, we should give them a "Release is in one week, we removed methods" notice on the forums when deleting or changing the signature of any existing TShock/TSAPI function without providing some sort of replacement stub.

QuiCM commented 9 years ago

Everything deprecated in TShock aside from Log.cs should, imo, be removed in the next release as they have been deprecated for months if not years now.

Olink commented 9 years ago

Great. Now answer the question, 'However, there is no requirement of this deprecation removal being made public to people who may be using the deprecated methods. Should we make this transparent by ensuring we give developers a week heads up on the release?"

QuiCM commented 9 years ago

Deprecations made from here on should be documented with their associated release, advising plugin developers to move away from them (so they can be removed in the following release). Current deprecations should not require this courtesy as there is no reason for current plugins to be using the currently deprecated methods.

Olink commented 9 years ago

I found 5 deprecations in a plugin today, only because i was curious about why I had 10 warnings. Its pretty clear that a lot of developers do not religiously check deprecation warnings.

We should replace deprecated function bodies with throw new IllegalOperationException("Beep Boop!");

AxisKriel commented 9 years ago

@Olink :+1:

hakusaro commented 9 years ago

I agree with @WhiteXZ -- it's not like we're teaching developers good practices by keeping deprecated methods in for them. If we babysit everyone with deprecations, we're setting a bad precedent, at least in my opinion.

If we want to give one more final "from this point out, this is our deprecation policy" warning, that's cool, but I want to resist from babying everyone with week notices here and there.

Olink commented 9 years ago

Ok, in the next release we purge all deprecated methods and post a warning notifying that all deprecations will be removed in the following release.