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

Flake8 linting of sydent/ and tests/ directories #345

Closed
wants to merge 11 commits into from

Conversation

H-Shay
Copy link
Contributor

@H-Shay H-Shay commented May 26, 2021

This PR is the result of running flake8 on all the .py files in the sydent/ directory (including subdirectories where applicable). It's a lot of changes which I sort of broke out and tried to group logically by committing all the modified files in each subdirectory together. I stopped before doing the tests/ directory and matrix_is_tester/ directory just because this seemed like a lot of changes and I also wanted to make sure I was doing this right-if not I didn't want to dig myself too big of a hole!

Just FYI I know the buildkite CI tests are gonna fail so i want to note that the unit tests are passing locally and that the matrix_is_tester tests are passing/failing in the same exact ratio they were before I started.

@richvdh richvdh requested a review from a team May 26, 2021 09:31
@callahad callahad self-assigned this May 26, 2021
@callahad
Copy link
Contributor

Sitting on this until we get the Black PR (#344) merged; will review then

@H-Shay
Copy link
Contributor Author

H-Shay commented May 27, 2021

Here's the version of these files-all flake8 errors were fixed (or muted where I thought appropriate) except for the F401 errors which I just wanted to check in about.

@H-Shay H-Shay changed the title Flake8 linting of sydent/ directory Flake8 linting of sydent/ and tests/ directory May 28, 2021
@H-Shay H-Shay changed the title Flake8 linting of sydent/ and tests/ directory Flake8 linting of sydent/ and tests/ directories May 28, 2021
@H-Shay
Copy link
Contributor Author

H-Shay commented May 28, 2021

Hopefully this is it for the flake8 linting! Tests/ only had a few small changes and matrix_is_tests had none so I think we are good here, except for the F401 errors question I had-once that's resolved I can update the PR and we should be good to go.

# name. The common name of the certificate we use for tests is fake.server.
config = {
"general": {"server.name": "fake.server"},
"crypto": {
"ed25519.signingkey": "ed25519 0 b29eXMMAYCFvFEtq9mLI42aivMtcg4Hl0wK89a+Vb6c"
"ed25519.signingkey": "ed25519 0 b29eXMMAYCFvFEtq9mLI42aivMtcg4Hl0wK89a+Vb6c" # noqa: E501
Copy link
Contributor

@clokep clokep May 28, 2021

Choose a reason for hiding this comment

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

It might make more sense to globally disable E501 like we do for Synapse (for the same reasoning, that black handles it for us).

@callahad callahad mentioned this pull request May 28, 2021
2 tasks
@callahad
Copy link
Contributor

Revised and continued in #347

@callahad callahad closed this May 28, 2021
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.

None yet

3 participants