Skip to content

SAM-147: black format all files#420

Closed
eapearson wants to merge 22 commits into
masterfrom
feature-SAM-147-black-format-all-files
Closed

SAM-147: black format all files#420
eapearson wants to merge 22 commits into
masterfrom
feature-SAM-147-black-format-all-files

Conversation

@eapearson

@eapearson eapearson commented Jan 13, 2022

Copy link
Copy Markdown
Contributor

This PR simply applies the black Python formatter to all source files. This should help avoid git-commit-diff spam when devs do their due diligence to ensure that all source files well-formed. It also paves the way for putting in place additional code quality checks, including whether the source files are properly formatted.

There are no functional changes to source files. Some cleanup was necessary after a merge from master which introduced merge conflicts, and the doc for pipenv usage was updated from an improved version from the SAM-96 work.

The formatting was done by simply installing the python dependencies locally using the pipenv instructions, then running the following commands, to format the code in the respective directories.

black lib
black test

@eapearson eapearson marked this pull request as ready for review January 13, 2022 17:38
@eapearson

Copy link
Copy Markdown
Contributor Author

We should have black-specific code quality checks in GHA workflows later. For now I think it is good enough if devs use the auto-formatting feature of their favorite IDE or Editor to ensure that files are always black-formatted when touched. Or one could always just enter black lib and black test to format relevant source files.

@lgtm-com

lgtm-com Bot commented Jan 14, 2022

Copy link
Copy Markdown

This pull request fixes 1 alert when merging 981568a into 3390914 - view on LGTM.com

fixed alerts:

  • 1 for Unused import

@lgtm-com

lgtm-com Bot commented Jan 14, 2022

Copy link
Copy Markdown

This pull request fixes 1 alert when merging 62b7b01 into 3390914 - view on LGTM.com

fixed alerts:

  • 1 for Unused import

@eapearson

Copy link
Copy Markdown
Contributor Author

Added support for using the official kb-sdk image.
Regenerated impl file using this image, so that it contains the proper malformatted placeholder comments.

@lgtm-com

lgtm-com Bot commented Jan 14, 2022

Copy link
Copy Markdown

This pull request fixes 1 alert when merging 6d371b6 into 3390914 - view on LGTM.com

fixed alerts:

  • 1 for Unused import

@lgtm-com

lgtm-com Bot commented Jan 25, 2022

Copy link
Copy Markdown

This pull request fixes 1 alert when merging dd3f029 into 27eb4d9 - view on LGTM.com

fixed alerts:

  • 1 for Unused import

@lgtm-com

lgtm-com Bot commented Feb 17, 2022

Copy link
Copy Markdown

This pull request fixes 1 alert when merging d5a04ab into e519c8b - view on LGTM.com

fixed alerts:

  • 1 for Unused import

@eapearson

Copy link
Copy Markdown
Contributor Author

@scanon I've resolved the merge conflicts, added a simple make task, added configuration to exclude generated files or files copied into lib (clients).

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.

2 participants