-
-
Notifications
You must be signed in to change notification settings - Fork 615
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
Updates to address docstring code scan issues, add flake8 configuration #671
Conversation
Signed-off-by: asears <asears@users.noreply.github.com>
Co-authored-by: Eric Brown <ericwb@users.noreply.github.com>
Signed-off-by: asears <asears@users.noreply.github.com>
Signed-off-by: asears <asears@users.noreply.github.com>
Signed-off-by: asears <asears@users.noreply.github.com>
bandit/blacklists/calls.py
Outdated
==================================================== | ||
Blacklist various Python calls known to be dangerous | ||
==================================================== | ||
Blacklist various Python calls known to be dangerous. |
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.
By removing the '===', this changes the font from a heading to normal paragraph text. However, this text is intended to be a heading. See https://bandit.readthedocs.io/en/latest/blacklists/blacklist_calls.html
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.
Reverting first line missing period warning update.
bandit/blacklists/imports.py
Outdated
====================================================== | ||
Blacklist various Python imports known to be dangerous | ||
====================================================== | ||
Blacklist various Python imports known to be dangerous. |
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.
By removing the '===', this changes the font from a heading to normal paragraph text. However, this text is intended to be a heading. See https://bandit.readthedocs.io/en/latest/blacklists/blacklist_calls.html
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.
Reverting first line missing period warning update.
Signed-off-by: asears <asears@users.noreply.github.com>
.flake8
Outdated
@@ -0,0 +1,2 @@ | |||
[flake8] | |||
max-line-length=120 |
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.
Almost all of the codebase is already conforming to a 80 char max line length (inherited from checks when in OpenStack). Even though 80 can be short at times, I think it would be best to keep the pattern. Thanks
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.
Reverting this check. 119 is the limit for Github PR's, will stay with OpenStack convention.
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.
Wondering if line too long warning should be ignored individually for those single-line docstring comments, or should the docstring be split up?
bandit\formatters\screen.py:98:80: E501 line too long (99 > 79 characters)
"""Output issue str returns a list of lines that should be added to the existing lines list."""
^
bandit\formatters\text.py:44:80: E501 line too long (103 > 79 characters)
"""Get verbose details including files in scope, score, SEVERITY and CONFLUENCE, files excluded."""
^
bandit\formatters\text.py:71:80: E501 line too long (99 > 79 characters)
"""Output issue str returns a list of lines that should be added to the existing lines list."""
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.
Yeah, we should line wrap those lines that are 80 or more. I already merged this commit, so feel free to create a new PR to handle above.
Signed-off-by: asears <asears@users.noreply.github.com>
Signed-off-by: asears <asears@users.noreply.github.com>
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.
LGTM
…on (PyCQA#671) * Updates to address docstring code scan issues, add flake8 configuration Signed-off-by: asears <asears@users.noreply.github.com> * Update .flake8 Co-authored-by: Eric Brown <ericwb@users.noreply.github.com> * Shorthand SPDX license one line header Signed-off-by: asears <asears@users.noreply.github.com> * update main for single-line SPDX and remove additional comment Signed-off-by: asears <asears@users.noreply.github.com> * revert init py docstrings. Signed-off-by: asears <asears@users.noreply.github.com> * Revert headings after PR review Signed-off-by: asears <asears@users.noreply.github.com> * remove 120 character limit setting Signed-off-by: asears <asears@users.noreply.github.com> * shorten description to address 80 character limit Signed-off-by: asears <asears@users.noreply.github.com> Co-authored-by: Eric Brown <ericwb@users.noreply.github.com>
…on (PyCQA#671) * Updates to address docstring code scan issues, add flake8 configuration Signed-off-by: asears <asears@users.noreply.github.com> * Update .flake8 Co-authored-by: Eric Brown <ericwb@users.noreply.github.com> * Shorthand SPDX license one line header Signed-off-by: asears <asears@users.noreply.github.com> * update main for single-line SPDX and remove additional comment Signed-off-by: asears <asears@users.noreply.github.com> * revert init py docstrings. Signed-off-by: asears <asears@users.noreply.github.com> * Revert headings after PR review Signed-off-by: asears <asears@users.noreply.github.com> * remove 120 character limit setting Signed-off-by: asears <asears@users.noreply.github.com> * shorten description to address 80 character limit Signed-off-by: asears <asears@users.noreply.github.com> Co-authored-by: Eric Brown <ericwb@users.noreply.github.com>
First time contributor. Flake8 and some other improvement libraries could be added to the project's requirements_test.txt and build process if required.
This changes adds some missing docstring values and flake8 configuration for 120 character limit and covers bandit/blacklist and bandit/cli. Flake8 yelps about capitalization and periods so addressed that too.
Signed-off-by: asears asears@users.noreply.github.com