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

Use version constant directly #985

Merged
merged 3 commits into from
Nov 12, 2024
Merged

Conversation

obenland
Copy link
Member

Since ACTIVITYPUB_PLUGIN_VERSION gets defined at the top of activitypub.php, get_plugin_version() will always return that and never fall back to the plugin meta information.

Not sure if 4.2.0 is the right version number in the deprecation documentation.
Not sure even if deprecating function is something this plugin does, or if it breaks back compat.

Proposed changes:

  • Deprecates Activitypub\get_plugin_version() and Activitypub\Migration::get_target_version() in favor of ACTIVITYPUB_PLUGIN_VERSION.
  • Replaces all uses of the deprecated function with the constant.

pfefferle
pfefferle previously approved these changes Nov 12, 2024
@obenland
Copy link
Member Author

@pfefferle Do you have any guidance on this?

Not sure if 4.2.0 is the right version number in the deprecation documentation.
Not sure even if deprecating function is something this plugin does, or if it breaks back compat.

@pfefferle
Copy link
Member

@obenland Oh, haven't seen that, sorry!

We follow semantic versioning, so fully removing the functions would have required a 5.0.0 release due to the breaking change. Since you retained the functions and added a deprecation warning, opting for the next possible minor version makes total sense.

@obenland
Copy link
Member Author

Do you have a preference one way or the other?

@pfefferle
Copy link
Member

pfefferle commented Nov 12, 2024

Exactly like you've done it: https://github.com/Automattic/wordpress-activitypub/pull/968/files#diff-2fb16a1f775688a0027ff486df52ce4bb27d1c754b878990408219982d3205b4 ☺️

Keeping the old files/classes/functions for a while, add deprecation message and remove them with the next major version.

Copy link
Member

@pfefferle pfefferle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense!

@obenland obenland merged commit ab22010 into trunk Nov 12, 2024
21 checks passed
@obenland obenland deleted the fix/plugin-version-constant-usage branch November 12, 2024 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants