Skip to content

refactor: deprecate StatusBase.register decorator#1384

Merged
james-garner-canonical merged 4 commits intocanonical:mainfrom
james-garner-canonical:status-base-init-subclass-deprecate-register
Sep 23, 2024
Merged

refactor: deprecate StatusBase.register decorator#1384
james-garner-canonical merged 4 commits intocanonical:mainfrom
james-garner-canonical:status-base-init-subclass-deprecate-register

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.

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
Collaborator

@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.

@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
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