-
Notifications
You must be signed in to change notification settings - Fork 9
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
Replace ASSERT statements with if statements #175 #60
base: main
Are you sure you want to change the base?
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.
Same as for your other PR: https://aboutcode.readthedocs.io/en/latest/contributing/writing_good_commit_messages.html
src/container_inspector/cli.py
Outdated
@@ -41,7 +41,7 @@ def container_inspector_squash(image_path, extract_directory): | |||
|
|||
def _container_inspector_squash(image_path, extract_directory): | |||
images = get_images_from_dir_or_tarball(image_path) | |||
assert len(images) == 1, 'Can only squash one image at a time' | |||
if(len(images) == 1): raise ValueError("Can only squash one image at a time") |
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.
no one liner please, and check your code formatting.
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.
sorry I thought if extra lines were added it might clash or smth.
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.
Used the imperative mood in the message and changed the code(No one liners).
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.
The tests are failing. The logic is not correct, as the assert is now inverted. Also please squash your commits and add missing DCO signoff. Please run the test and review the code you are pushing twice before pushing. ;)
@@ -41,7 +41,8 @@ def container_inspector_squash(image_path, extract_directory): | |||
|
|||
def _container_inspector_squash(image_path, extract_directory): | |||
images = get_images_from_dir_or_tarball(image_path) | |||
assert len(images) == 1, 'Can only squash one image at a time' | |||
if(len(images) == 1): |
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.
Is the code formatted? Is this if statement correct?
You need to have a hard look, and if need be, also add a test that passes before making this change.
Replace ASSERT statements with if statements #175
Reported by: pombredanne aboutcode-org/aboutcode#175
Replace assert with explicit exception handling
Raise ValueError if inputs are invalid
Signed-off-by: Shafi456 [email protected]