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

improve cmd deprecation warning by support nested replaced_by_cmd #282

Merged
merged 1 commit into from
Apr 25, 2024

Conversation

mjurbanski-reef
Copy link

No description provided.

@mjurbanski-reef mjurbanski-reef added the -changelog skip changelog label Apr 25, 2024
@@ -1121,12 +1121,12 @@ def __str__(self):

class CmdReplacedByMixin:
deprecated = True
replaced_by_cmd: type[Command]
replaced_by_cmd: type[Command] | tuple[type[Command], ...]

def run(self, args):
self._print_stderr(
f'WARNING: {self.__class__.name_and_alias()[0]} command is deprecated. '
Copy link
Author

Choose a reason for hiding this comment

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

I do realize I dont handle the case of replacing nested command, only BY nested commands... but I don't think it matters much right now and alternative would require figuring out the registry, building path from that etc.

Worst case scenario we use this solution for this release, and rewrite these few lines of code in the next one IF need arises.

@mjurbanski-reef mjurbanski-reef force-pushed the nested_replaced_by_cmd branch from 467eba1 to 589d49a Compare April 25, 2024 07:49
@mjurbanski-reef mjurbanski-reef force-pushed the nested_replaced_by_cmd branch from 589d49a to 1c00f90 Compare April 25, 2024 08:48
@adal-chiriliuc-reef adal-chiriliuc-reef merged commit a11c1b6 into master Apr 25, 2024
30 checks passed
@adal-chiriliuc-reef adal-chiriliuc-reef deleted the nested_replaced_by_cmd branch April 25, 2024 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-changelog skip changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants