Skip to content
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

Add more controls in pre-commit, partly security #1036

Open
wants to merge 28 commits into
base: master
Choose a base branch
from

Conversation

juju4
Copy link
Contributor

@juju4 juju4 commented Nov 19, 2022

By using Black, you agree to cede control over minutiae of hand-formatting. In return, Black gives you speed, determinism, and freedom from pycodestyle nagging about formatting. You will save time and mental energy for more important matters.

@achantavy
Copy link
Contributor

Hi - It'd be easier to discuss these additions one by one since adding them all at once will be a big change to our CI. Is there one you'd like us to consider first?

@juju4
Copy link
Contributor Author

juju4 commented Jan 7, 2023

order does not really matter if ending to merge all.

codespell is about code spelling
detect-secrets try to prevent committing secrets in code
semgrep and bandit are security code scanning tools

example run
https://github.com/juju4/cartography/actions/runs/3863435878/jobs/6585623944#step:4:118

@ben-elttam
Copy link

Choosing to use black will mean a big patch at first to get the code in the black style formatting, but going forward will make PRs more straight forward as the formatting style is black.

@chandanchowdhury
Copy link
Collaborator

The MR looks large/scary, however 90% of the changes are strictly cosmetic e.g. fix spelling and lint error etc.

Notes:

  • Not enabling black yet.
  • Added TIMEOUT to requests call in two files. Modules impacted: Azure, Github.
  • Forced TLS verification in one file. Modules impacted: BigFix.
  • Spelling error fixed for one variable in two files under AWS test.
  • Use yaml.SafeLoader instead of yaml.FullLoader. Module imapced: AWS
  • Strictly cosmetic (spelling, lint etc.) changes in twenty six files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants