-
Notifications
You must be signed in to change notification settings - Fork 113
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
Cleanup CLI #255
Cleanup CLI #255
Conversation
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!
@@ -32,17 +32,6 @@ def initialize( | |||
@relative_file_set = relative_file_set | |||
end | |||
|
|||
sig { returns(Result) } | |||
def detect_stale_violations |
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.
Let's deprecate this command. It was added by our team long time ago, we don't use it, but nothing prevents people from using it. At least we should deprecate in the stable branch before our next release and remove on main.
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.
This was my hesitation with merging. I made sure we weren't using it, but I don't know about others. We could just kill them in the next major release, but deprecating this one sounds good to me.
aeebd53
to
ed2e247
Compare
init does the same thing and this command is not documented.
Makes command parsing where clause easier to read.
This command's output is in check and isn't documented.
ed2e247
to
6eeac84
Compare
What are you trying to accomplish?
Clean up the CLI class.
What approach did you choose and why?
Clean up the CLI to declutter the command parsing where clause and clean up / deprecate old commands that aren't documented.
What should reviewers focus on?
This removes the
generate_configs
subcommand and deprecates thedetect-stale-violations
subcommand.Type of Change
Additional Release Notes
This might be a breaking change if anyone is using the undocumented commands. Since we don't mention them anywhere, I think they are safe to delete.
Checklist