-
Notifications
You must be signed in to change notification settings - Fork 197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Give deprecation warning on old interfaces #479
Comments
Frankly I don't see this as high priority. Sure if you want to add it, go ahead but keep it short. Using these class names outside the library is unusual and I think the documentation doesn't even mention them in any guide or example. So the usage will be very low and I expect anyone depending somewhat on the library internals to do proper testing or at least read the change log thoroughly before upgrading a dependency. As for the compatibility aliases, they don't bother me at all where they currently live. So there's no planned time frame to remove them. |
Any (active developed) API will evolve over time. It is wise to have a strategy of how to introduce new methods and classes, how to deprecate and then finally remove obsolete ones:
|
I guess each of these bullet points need someone to put in the work. If you're up to it, make some suggestions. So far we simply don't have a "grand plan" about deprecation, so I'd handle it lazily until it gets annoying somewhere. I love software that just keeps working and doesn't break other code arbitrarily. Keeping the old names is the simplest approach to that as it requires no effort really. |
We should probably implement deprecation warnings in the code. We have changed a few class names recently and we have code in place for compatibility. E.g.
canopen.sdo.base
definesRecord
as alias toSdoRecord
and so on. I agree that we must not introduce breaking change and have aliases like this, but I think we should inform them as deprecated so we steer the users towards the new API.We should also write a statement when these aliases will stop working.
The text was updated successfully, but these errors were encountered: