Skip to content
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

Deprecation warnings #305

Open
autonordev opened this issue Aug 27, 2023 · 0 comments
Open

Deprecation warnings #305

autonordev opened this issue Aug 27, 2023 · 0 comments
Assignees
Milestone

Comments

@autonordev
Copy link
Collaborator

autonordev commented Aug 27, 2023

Cmdr itself is over five years old, some of the older parts of the code (like the interface) are even older. By this time next month, it'll have been half a decade since Cmdr 1.0.0 released.

Features have been added, removed and changed. In order to sustain our commitment to Cmdr's long-term reliability and minimise any pain that may occur in the unlikely event of a major update (2.0.0), we need a reliable system to allow developers to be warned when they're using outdated (deprecated) methods or relying on deprecated behaviour.

Deprecation warnings will:

  • allow developers to know when they're using methods or behaviour which could be broken in a major update
  • allow us to better identify what can be removed in a major update
  • discourage developers from using deprecated features

How deprecation warnings work

It's TBC which component will be responsible for handling them, probably Util, but there'll be private methods to register and to emit a deprecation warning. Deprecation warnings must be registered without delay to prevent false errors in developer code.

Deprecation warnings should be descriptive and written in PascalCase, for example AddHookMethod. They may be then accompanied by a longer description, which'll be emitted in the warning.

Example:

Util:RegisterDeprecationWarning("AddHookMethod", "This method was renamed to RegisterHook in v1.1.2. The old name exists for backwards compatibility and should not be used for new work.")

function Registry:AddHook(...)
	Util:EmitDeprecationWarning("AddHookMethod")
	Registry:RegisterHook(...)
end

This would present in the output:

[Cmdr] [DeprecationWarning::AddHookMethod] This method was renamed to RegisterHook in v1.1.2. The old name exists for backwards compatibility and should not be used for new work.
(stack trace to Registry:AddHook)

Suppression of deprecation warnings

A Util:SuppressDeprecationWarning method would exist to silence a deprecation warning. A deprecation warning may also be silenced by default if - for instance - it is no longer required. This method may be proxied into the Cmdr and CmdrClient singletons (TBC).

The method would accept either a string (to suppress a single warning) or an array of strings (to suppress multiple warnings).

Example:

Util:SuppressDeprecationWarning("AddHookMethod")
Util:SuppressDeprecationWarning({ "AddHookMethod", "GetCommandsAsStringsMethod" })

Calling with an invalid deprecation warning name (i.e. an unregistered one) would emit an error.

What should have deprecation warnings

As above, the AddHook and GetCommandsAsStrings methods on the Registry are good places to start.

The metatable 'shadowing' behaviour (server -> registry, client -> dispatcher) can be confusing and should be distracted; this'll also encourage developers to directly interact with the components they want to use and help reduce support requests.

We should also be on the lookout for other deprecated behaviours or methods, and ensure they're warned appropriately.

Versioning policy

While a deprecation can technically be included within a patch, the inclusion of a deprecation warning needs to be in a minor version.

Deprecation warnings cannot be removed in a patch or minor version, only a major version, although they may be silenced by default in a patch version or greater.

With a new major version, all deprecated elements should be removed, and all deprecation warnings should be removed accordingly.

Future

In the future, we could make it possible for developers to programmatically listen to deprecation warnings. Although there are currently no such plans, implementation of this would therefore need to be described and approved in a new issue ticket, after deprecation warnings have been released.

@autonordev autonordev self-assigned this Aug 27, 2023
@autonordev autonordev added this to the 1.13 milestone Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant