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 black and isort #1115

Merged
merged 4 commits into from
Jun 24, 2020
Merged

Add black and isort #1115

merged 4 commits into from
Jun 24, 2020

Conversation

rmol
Copy link
Contributor

@rmol rmol commented Jun 16, 2020

Description

Adds the black and isort tools to the development requirements, and adds Makefile targets for both checking and applying them.

Fixes #1100.

Test Plan

  • Check out the first commit on add-black-isort with git checkout 86ee815
  • Run make check-isort. It should show the changes it would make.
  • Run make check-black. It should show the changes it would make.
  • Check out the last commit: git checkout add-black-isort
  • Rerun make check-isort and make check-black. No changes should be indicated.
  • Run git blame securedrop_client/logic.py | head -60 | grep -c 361ba88. There should be 22 lines attributed to that revision.
  • Run:
    • git config blame.ignoreRevsFile .git-blame-ignore-revs
    • git config blame.markIgnoredLines true
    • git config blame.markUnblamableLines true
  • Run git blame securedrop_client/logic.py | head -60 | grep 361ba88. There should be three lines attributed to that revision, each prefixed with an asterisk. This indicates that those changes are only attributable to the ignored revision.

Checklist

If these changes modify code paths involving cryptography, the opening of files in VMs or network (via the RPC service) traffic, Qubes testing in the staging environment is required. For fine tuning of the graphical user interface, testing in any environment in Qubes is required. Please check as applicable:

  • I have tested these changes in the appropriate Qubes environment
  • I do not have an appropriate Qubes OS workstation set up (the reviewer will need to test these changes)
  • These changes should not need testing in Qubes

If these changes add or remove files other than client code, the AppArmor profile may need to be updated. Please check as applicable:

  • I have updated the AppArmor profile
  • No update to the AppArmor profile is required for these changes
  • I don't know and would appreciate guidance

If these changes modify the database schema, you should include a database migration. Please check as applicable:

  • I have written a migration and upgraded a test database based on master and confirmed that the migration applies cleanly
  • I have written a migration but have not upgraded a test database based on master and would like the reviewer to do so
  • I need help writing a database migration
  • No database schema changes are needed

@eloquence
Copy link
Member

eloquence commented Jun 16, 2020

This is still blocked on the pypi.securedrop.org update, correct?

Is this blocked on the same packaging change described in freedomofpress/securedrop-sdk#124 (comment) ?

@rmol
Copy link
Contributor Author

rmol commented Jun 17, 2020

Technically not blocked -- the formatting changes can be verified -- but we do need to change the Debian package rules in securedrop-debian-packaging before we can successfully package it.

@rmol
Copy link
Contributor Author

rmol commented Jun 17, 2020

Um, I should check CI before talking. Yeah, this isn't getting through CI until the packaging repo is updated. I'll see to that first thing in the morning.

Copy link
Contributor

@kushaldas kushaldas left a comment

Choose a reason for hiding this comment

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

The change looks good, I will go through it once more after the CI is green.

@kushaldas kushaldas self-assigned this Jun 22, 2020
@eloquence
Copy link
Member

@rmol Flagging that this branch now has conflicts.

@rmol rmol force-pushed the add-black-isort branch from 95f2b47 to 137a86c Compare June 23, 2020 22:06
@rmol
Copy link
Contributor Author

rmol commented Jun 23, 2020

@eloquence Thanks, rebased.

@rmol rmol force-pushed the add-black-isort branch from 137a86c to bba9f9d Compare June 23, 2020 22:14
@rmol
Copy link
Contributor Author

rmol commented Jun 23, 2020

And again. 😐

Copy link
Contributor

@kushaldas kushaldas left a comment

Choose a reason for hiding this comment

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

Trying to approve. This is good.

@kushaldas kushaldas merged commit 6f70b18 into master Jun 24, 2020
@kushaldas kushaldas deleted the add-black-isort branch June 24, 2020 16:02
@sssoleileraaa sssoleileraaa mentioned this pull request Jul 16, 2020
2 tasks
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.

add black and isort formatting tools
3 participants