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

Fix actions #1359

Merged
merged 29 commits into from
Aug 24, 2021
Merged

Fix actions #1359

merged 29 commits into from
Aug 24, 2021

Conversation

aryaniyaps
Copy link
Contributor

@aryaniyaps aryaniyaps commented Aug 21, 2021

With the previous PR where we aimed to fix CI with github actions, the workflows had a few problems. This PR aims to fix that.

What's been added/modified:

  • split the test workflow into three separate workflow (tests, coveralls, deploy).
  • the deploy workflow fires conditionally, and not with every push/ pull request.
  • add lint workflow and update deploy referencing the v2 branch (which was previously setup by @jkimbo )
  • fix the location of stale.yml
    Also, python 3.8 is being primarily used, because it's the most stable version we have yet (over 3.9)

Also removes unused imports from test files.

@aryaniyaps aryaniyaps marked this pull request as draft August 21, 2021 13:02
@aryaniyaps
Copy link
Contributor Author

@jkimbo @syrusakbary what do you suggest we do with the pre-commit hooks? Should they alter files when we commit (this might be a large change because every file is checked). Right now, the PR checks fail if there is any issue with the commit (like improper formatting or old python code).
We could alter the pre-commit hooks to just magically format and upgrade code on each commit, if that is preferred.

@aryaniyaps
Copy link
Contributor Author

aryaniyaps commented Aug 22, 2021

I'm also having trouble running pre-commit on my machine. black formatter fails because of Errno 13, as well as pyupgrade for some reason. I will try to fix this later on another machine.

@syrusakbary
Copy link
Member

Great work on the PR @codebyaryan.

The action should just check if the lint is correct, but don't update the code automatically (that should be done on the user side before pushing).
Could you run the linter locally and commit also the files with the formatted fixes?

Once that's done we should be good to merge!

@aryaniyaps
Copy link
Contributor Author

Thanks for the reply! I will make this PR ready ASAP!

@aryaniyaps aryaniyaps marked this pull request as ready for review August 24, 2021 03:01
@aryaniyaps
Copy link
Contributor Author

aryaniyaps commented Aug 24, 2021

shouldn't the continuous-integration/travis-ci action be removed? (because we switched to github actions instead)
also, sorry for a load of commits. I couldn't access my local editor earlier so pushed changes directly from the website (lol?)

@syrusakbary
Copy link
Member

shouldn't the continuous-integration/travis-ci action be removed? (because we switched to github actions instead)

Just fixed it!

@syrusakbary syrusakbary merged commit f039af2 into graphql-python:master Aug 24, 2021
@aryaniyaps aryaniyaps deleted the fix-actions branch August 24, 2021 03:29
@ulgens
Copy link
Contributor

ulgens commented Aug 24, 2021

@syrusakbary I know it's too late but it would be nicer if we squashed this before merging 🙂

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.

3 participants