-
Notifications
You must be signed in to change notification settings - Fork 148
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
chore/deprection message for license command #614
chore/deprection message for license command #614
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.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
safety/cli.py (1)
241-275
: Update docstring to be command-agnostic and add input validation.The function implementation is good, but there are a few improvements needed:
- The docstring specifically mentions the 'check' command, but the function is generic.
- Missing input validation for the deprecation_date parameter.
Apply these changes:
def print_deprecation_message(old_command, deprecation_date, new_command=None): """ - Print a formatted deprecation message for the 'check' command. + Print a formatted deprecation message for a deprecated command. This function uses the click library to output a visually distinct message in the console, warning users about the deprecation of the - 'check' command. It includes information about the deprecation date + specified command. It includes information about the deprecation date and suggests an alternative command to use, if provided. The message is formatted with colors and styles for emphasis: - Yellow for the border and general information - Red for the 'DEPRECATED' label - Green for the suggestion of the new command (if provided) Parameters: - old_command (str): The name of the deprecated command. - deprecation_date (datetime): The date when the command will be deprecated. - new_command (str, optional): The name of the new command to suggest. Default is None. + + Raises: + - ValueError: If deprecation_date is None or not a valid date object. """ + if not deprecation_date: + raise ValueError("deprecation_date cannot be None") + if not isinstance(deprecation_date, date): + raise ValueError("deprecation_date must be a date object")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- .vscode/launch.json (1 hunks)
- safety/cli.py (5 hunks)
🧰 Additional context used
🔇 Additional comments (2)
.vscode/launch.json (1)
35-44
: LGTM! Well-structured debug configuration.The new "Safety License" configuration follows the established pattern and will be useful for testing the deprecated license command functionality.
safety/cli.py (1)
486-486
: Verify if there's a replacement command for the license functionality.The deprecation message for the license command doesn't suggest an alternative command. If there's a new command that will replace this functionality, consider adding it to the deprecation message.
✅ Verification successful
Based on the search results, I can now generate the final response since I have found clear evidence about the future of the license command in the codebase.
Deprecation message correctly indicates no direct replacement command
The codebase shows that the license functionality will be integrated into the
safety scan
command, as indicated by the help text constantCLI_LICENSES_COMMAND_HELP
which states: "This command will be replaced by safety scan, and will be unsupported beyond 1 May 2024."The current implementation of
print_deprecation_message("license", date(2024, 6, 1), new_command=None)
is correct because:
- The functionality is being merged into an existing command (
safety scan
) rather than having a direct 1:1 replacement command- The help text already informs users about the transition to
safety scan
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for potential new license-related commands in the codebase rg -i "license.*command" --type pyLength of output: 1016
a8d55ac
to
c07afc5
Compare
c07afc5
to
1051622
Compare
WalkthroughThis pull request introduces a new debugging configuration for the "Safety License" functionality in the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (6)
.vscode/launch.json (1)
35-44
: LGTM! Consider adding a deprecation note in the configuration.The debug configuration is well-structured and consistent with other configurations. However, since the license command is being deprecated, it would be helpful to add a comment indicating its deprecated status.
{ + // Note: This configuration is for the deprecated license command "name": "Safety License", "type": "debugpy", "request": "launch",
safety/constants.py (1)
188-188
: Consider improving the BAR_LINE constant's documentation and readabilityWhile the constant serves its purpose for formatting deprecation messages, consider:
- Adding a docstring to explain its purpose and usage
- Using a more maintainable approach to define the line length
-BAR_LINE = "+===========================================================================================================================================================================================+" +# Length of 123 "=" characters enclosed in "+" symbols +BAR_LINE = "+" + "=" * 123 + "+"Also consider adding a docstring:
# Add above the BAR_LINE constant """Formatting constant used to create visual separation in deprecation messages. The line is 125 characters wide (123 "=" chars + 2 "+" chars) to ensure proper alignment in terminal output. """safety/cli.py (4)
256-259
: Correct the parameter type in the docstringIn the docstring, update the type of
deprecation_date
fromdatetime
todate
to match the type annotation and usage.Apply this diff to correct the docstring:
Parameters: - old_command (str): The name of the deprecated command. - - deprecation_date (datetime): The date when the command will no longer be supported. + - deprecation_date (date): The date when the command will no longer be supported. - new_command (str, optional): The name of the alternative command to suggest. Default is None.
353-353
: Fix typo in docstring: 'enviroment' should be 'environment'There's a typographical error in the docstring. Correct 'enviroment' to 'environment'.
Apply this diff to fix the typo:
""" [underline][DEPRECATED][/underline] `check` has been replaced by the `scan` command, and will be unsupported beyond 1 May 2024.Find vulnerabilities at a target file or enviroment. + [underline][DEPRECATED][/underline] `check` has been replaced by the `scan` command, and will be unsupported beyond 1 May 2024. Find vulnerabilities at a target file or environment. """
483-485
: Add deprecation notice to thelicense
command docstringThe
license
command is deprecated, but its docstring does not reflect this. Update the docstring to inform users about the deprecation.Apply this diff to update the docstring:
def license(ctx, db, output, cache, files): - """ - Find the open source licenses used by your Python dependencies. - """ + """ + [DEPRECATED] The `license` command will be unsupported beyond 1 June 2024. + Find the open source licenses used by your Python dependencies. + """
238-272
: Enhance the message formatting inprint_deprecation_message
Consider capitalizing the first letter of the message for better readability.
Apply this diff to adjust the message:
click.echo(click.style("DEPRECATED: ", fg="red", bold=True) + - click.style(f"this command (`{old_command}`) has been DEPRECATED, and will be unsupported beyond {deprecation_date.strftime('%d %B %Y')}.", fg="yellow", bold=True)) + click.style(f"This command (`{old_command}`) has been DEPRECATED, and will be unsupported beyond {deprecation_date.strftime('%d %B %Y')}.", fg="yellow", bold=True))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- .vscode/launch.json (1 hunks)
- safety/cli.py (7 hunks)
- safety/constants.py (1 hunks)
🧰 Additional context used
🔇 Additional comments (1)
safety/constants.py (1)
185-186
: LGTM: Exit code addition looks goodThe
EXIT_CODE_EMAIL_NOT_VERIFIED
constant:
- Follows the established pattern for exit codes
- Uses a unique value that doesn't conflict with existing codes
- Is properly spaced with a newline
34b73a5
to
311bbd9
Compare
147cdb1
to
4149b70
Compare
Description
Adds depreciated message for license command
Before:
Type of Change
Testing
Checklist
Summary by CodeRabbit
New Features
check
command to thescan
command.Bug Fixes
Documentation
Chores
BAR_LINE
for formatting purposes in output messages.EXIT_CODE_EMAIL_NOT_VERIFIED
to ensure correct definition.