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

refactor: deprecate StatusBase.register decorator #1384

Conversation

james-garner-canonical
Copy link
Contributor

@james-garner-canonical james-garner-canonical commented Sep 22, 2024

As StatusBase.__init_subclass__ is now used to automatically register status classes by name, and the old StatusBase.register decorator (1) shouldn't have been public, (2) shouldn't have been used externally anyway, (3) doesn't appear to be used in charms currently, and (4) isn't needed internally, let's deprecate it.

See also discussion on #1380.

ops/model.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@benhoyt benhoyt left a comment

Choose a reason for hiding this comment

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

Works for me, thanks!

Copy link
Contributor

@tonyandrewmeyer tonyandrewmeyer left a comment

Choose a reason for hiding this comment

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

LGTM! Small test suggestion and a question for Ben.

ops/model.py Outdated Show resolved Hide resolved
test/test_model.py Show resolved Hide resolved
@james-garner-canonical james-garner-canonical force-pushed the status-base-init-subclass-deprecate-register branch from 0782be2 to 75eb180 Compare September 22, 2024 23:12
@james-garner-canonical james-garner-canonical merged commit 98a4e4f into canonical:main Sep 23, 2024
29 checks passed
tonyandrewmeyer pushed a commit to tonyandrewmeyer/operator that referenced this pull request Oct 4, 2024
As `StatusBase.__init_subclass__` is now used to automatically register
status classes by name, and the old `StatusBase.register` decorator
shouldn't have been public in the first place, it is now deprecated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants