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

Run CI on GitHub Actions #413

Merged
merged 34 commits into from
Dec 30, 2021
Merged

Run CI on GitHub Actions #413

merged 34 commits into from
Dec 30, 2021

Conversation

elacuesta
Copy link
Member

@elacuesta elacuesta commented Dec 2, 2021

The idea is to stick to the title and avoid overlapping with #407 as much as possible, but a few things might be duplicated (Flake8 adjustments and Python classifiers, for instance).

  • Flake8 job (small adjustments)
  • Tests (Python 3.6 to 3.9; Ubuntu, macOS, Windows)
  • Readme badges
  • Removed Python 2.7 and 3.5 classifiers
  • PyPI publishing job
  • Add PyPI token
  • Attach artifacts to (draft) release (binaries created with the "freeze" tox environment)

Commit history is not very clean, it should be squash-merged.

@elacuesta elacuesta added the ci label Dec 2, 2021
@codecov
Copy link

codecov bot commented Dec 6, 2021

Codecov Report

Merging #413 (908c0a1) into master (51a53b0) will decrease coverage by 1.94%.
The diff coverage is n/a.

❗ Current head 908c0a1 differs from pull request most recent head 1b668e4. Consider uploading reports for the commit 1b668e4 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #413      +/-   ##
==========================================
- Coverage   91.42%   89.48%   -1.95%     
==========================================
  Files          34       34              
  Lines        2415     2415              
==========================================
- Hits         2208     2161      -47     
- Misses        207      254      +47     
Impacted Files Coverage Δ
shub/deploy_egg.py 82.53% <0.00%> (-14.29%) ⬇️
shub/compat.py 52.17% <0.00%> (-13.05%) ⬇️
shub/utils.py 82.39% <0.00%> (-6.77%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5fcb15f...1b668e4. Read the comment docs.

@elacuesta elacuesta changed the title [WIP] Run CI on GitHub Actions Run CI on GitHub Actions Dec 16, 2021
@elacuesta elacuesta marked this pull request as ready for review December 16, 2021 14:00
@@ -56,6 +56,7 @@ def test_can_clone_a_git_repo_and_deploy_the_egg(self):

self.assertTrue('master' in data['version'])

@unittest.skip('flaky')
Copy link
Member Author

Choose a reason for hiding this comment

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

This test works when executed on its own, but fails when executed as part of the whole test suite. While it would be great to know exactly what causes this, at this point I think it's more important to restore CI to a functioning state.

Copy link
Member

@Gallaecio Gallaecio left a comment

Choose a reason for hiding this comment

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

The code looks good to me, my only concern is one I expressed in #407:

I’m not sure about dropping Python support. I think it may be safer to do this after the rebranding.

It may not be a valid concern anymore, though.

Does Scrapy Cloud still support those removed Python versions? If so, we could still go ahead with this change, but we should probably include the python version requirement into setup.py, so that users with a modern pip get a version of shub compatible with Python 2, and also update documentation resources about installing shub to take into account the possibility that users have an old pip and hence they should manually pin the shub version in the pip install command.

@elacuesta elacuesta merged commit ea207cc into master Dec 30, 2021
@elacuesta elacuesta deleted the ci-github-actions branch December 30, 2021 15:25
elacuesta added a commit that referenced this pull request Dec 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants