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 code linters and remove dead code #44

Closed
wants to merge 0 commits into from

Conversation

aaaandrzej
Copy link
Contributor

This addresses some of the points mentioned in [#33]:

  • Fix flake8 errors
  • removing dead code

Black should format the code in a way acceptable by flake8 when using current config. make lint-fix to be run against this branch once #41 is merged to avoid too many conflicts.

I probably won't be able to fix points 3-5 very soon so the question is if we want to merge it separately or whether the PR should fix all of the [#33] issues

@ClasherKasten
Copy link
Collaborator

Some little things:

  • I would like to stick to single quotes instead of switching to double quotes
  • Makefile is an idea, but setting up pre-commit would be a better option in my opinion

@aaaandrzej
Copy link
Contributor Author

Single quotes seem doable, there's axblack fork of black that does that, I can add it easily. I won't be able to help with pre-commit hooks soon though, I'm afraid, as it looks like quite a big task itself. Would you accept a PR with formatting fixed, flake8 happy and dead code removed, or would you rather prefer for someone else to complete the whole [#33] at one go?

@aaaandrzej
Copy link
Contributor Author

Note on the PR closure - I underestimated merge conflicts and decided to force push which closed this PR automatically, but my previous comment should be still valid.

@ClasherKasten
Copy link
Collaborator

ClasherKasten commented Oct 8, 2022

Would you accept a PR with formatting fixed, flake8 happy and dead code removed, or would you rather prefer for someone else to complete the whole [https://github.com//issues/33] at one go?

Every single step towards better code quality is welcome. So yes, I would also merge a PR with formatting corrected, happy flake8 and dead code removed.

I won't be able to help with pre-commit hooks soon though, I'm afraid, as it looks like quite a big task itself.

Then I'll open a new issue for this.

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