-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Feat/consolidate cast convert #12638
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
base: master
Are you sure you want to change the base?
Feat/consolidate cast convert #12638
Conversation
…eat/consolidate-cast-convert
grandizzy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you will need to fix tests to move forward
Updated |
grandizzy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, ty, that's much cleaner. pending other reviewers as this is a breaking change and need consensus.
|
@dipanshuhappy appreciate the work ser |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should merge this as is as it is breaking. What I would suggest is shimming the existing commands to call convert <...> internally and displaying a "This command is deprecated and may be removed in the future, please use cast convert <...> instead" message.
I realize this ends up making the code a bit messy again, but I'd much prefer this to us pushing a breaking change for a very common set of commands in a non-major bump
Understood... Updating to that ASAP. |
Motivation
Resolves #12500
Solution
Create a new file
convert.rswhich encapsulates all the sub commands mentioned.PR Checklist
NOTE: This does break existing cli commands related to conversation. Refer to the issue to know more about the cli commands that will break.