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

Gh action to lint python and auto fix issues #87

Closed
wants to merge 0 commits into from

Conversation

mohitdmak
Copy link
Collaborator

Hello @jakebeal , this PR hopefully fixes #68

  • This currently uses flake8 and black linters.
  • In case of style issues, a new commit is automatically made fixing changes recommended by black

Do we need to proceed with recommended code styles by these linters, or:

  • Lint code by customizing lint rules?

@jakebeal
Copy link
Contributor

Thank you for the contribution!

Three thoughts regarding your question:

  1. I notice that flake8 is declaring a failure, but the code is passing in the action?
  2. It looks to me like it's being a little bit heavy-handed on the formatting --- can we dial it down to be less sensitive (e.g., just PEP8 compliance)?
  3. I'm not sure I'm comfortable doing auto-reformatting. I'd prefer to have it declare PEP8 violation errors and have the person pushing responsible for fixing.

@mohitdmak
Copy link
Collaborator Author

Regarding

  1. Actually, gh actions don't support auto commits or annotations on PRs from a forked repo, it is referenced by the creator of this action here: link . I have tried the workaround mentioned here by triggering the actions on pull_request_target events which should allow annotations on non-local PRs.
  2. This should be possible, would look into it.
  3. Sure, I disabled the auto committing, and once you run the workflows we can test if the annotations have started working here.

@jakebeal
Copy link
Contributor

I guess I don't know what you mean by an annotation --- I thought we would see a failure due to linting errors.

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 python linter
2 participants