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

Format the codebase #23

Merged
merged 4 commits into from
Aug 18, 2020
Merged

Format the codebase #23

merged 4 commits into from
Aug 18, 2020

Conversation

AlecRosenbaum
Copy link
Contributor

@AlecRosenbaum AlecRosenbaum commented Aug 17, 2020

Related: #12

This change:

  • adds a requirements_lint.txt file for linter requirements
  • formats code with black
  • adds a static-code-analysis job to CircleCI, which enforces code formatting

This is a pretty big diff which murders git history, but the files look much prettier now IMO. Note that the formatting is done with the default black settings. This means there's a line-length of 88 and strings are normalized.

@AlecRosenbaum AlecRosenbaum marked this pull request as ready for review August 17, 2020 14:52
@AlecRosenbaum AlecRosenbaum requested a review from thomasst August 17, 2020 14:59
@jkemp101
Copy link
Member

Do we want to switch to double quotes for this repo?

@AlecRosenbaum
Copy link
Contributor Author

I think so. Trying to adhere to a formatting convention explicitly not supported by black is not great IMO. In other projects where quotes aren't enforced due to the single quote convention, I have spent what felt like completely wasted time manually going back through changes and swapping quotes.

Copy link
Member

@thomasst thomasst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not review the specific formatting changes done by black, but enforcing default black settings SGTM.

@AlecRosenbaum AlecRosenbaum merged commit 9022236 into master Aug 18, 2020
@AlecRosenbaum AlecRosenbaum deleted the code-formatting branch August 18, 2020 12:27
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.

3 participants