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 github actions for CI #17

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add github actions for CI #17

wants to merge 4 commits into from

Conversation

Minion3665
Copy link
Member

  • When a new PR is submitted, autoformat code with autopep8 and remove
    commented out code
  • After reformatting, commit back to the pull request
  • When reformatted code has been committed, lint it with pycodestyle

On github side we will

  • Require all changes to be submitted as PRs
  • Require all CI checks to pass before merging PRs

This change partially-fixes #1 and fixes #10

- When a new PR is submitted, autoformat code with autopep8 and remove
  commented out code
- After reformatting, commit back to the pull request
- When reformatted code has been committed, lint it with pycodestyle

On github side we will
- Require all changes to be submitted as PRs
- Require all CI checks to pass before merging PRs

This change partially-fixes #1 and fixes #10
- This will be needed for CI later
- This is required for the python setup stage of my github action (act
  didn't catch that locally)
- The one I was using previously did not support pull requests
- If we do, the linter will still have the old environment, so it will
  be ineffective
@shardros
Copy link
Member

@Minion3665 I have made the repo public

@fenjalien
Copy link
Collaborator

Looks good, not entirely sure about eradicate as it may give a false positive but I'll trust it.

You could use the command poetry install --only dev instead. This will install all dev dependencies which can then be run. Or place all linting/reformatting in its own dependency group (https://python-poetry.org/docs/master/managing-dependencies/) and use the command poetry install --only reformat-lint.

@Minion3665
Copy link
Member Author

Looks good, not entirely sure about eradicate as it may give a false positive but I'll trust it.

You could use the command poetry install --only dev instead. This will install all dev dependencies which can then be run. Or place all linting/reformatting in its own dependency group (python-poetry.org/docs/master/managing-dependencies) and use the command poetry install --only reformat-lint.

I'll take a look at using poetry.

I think even if eradicate does give an occasional false positive it isn't much of an issue as we'll still have the old commits in our history, so I'm not too worried about the chance of that happening

@fenjalien
Copy link
Collaborator

I'll take a look at using poetry.

Don't worry too much about using poetry but add the packages used to the dev dependencies so they could be run/pre-checked before pushing.

I think even if eradicate does give an occasional false positive it isn't much of an issue as we'll still have the old commits in our history, so I'm not too worried about the chance of that happening

That would be annoying to do but fair

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.

Autopep8 formatting Setup CI
3 participants