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

Try GitHub Actions CI #70

Merged
merged 15 commits into from
Aug 31, 2019
Merged

Try GitHub Actions CI #70

merged 15 commits into from
Aug 31, 2019

Conversation

hugovk
Copy link
Owner

@hugovk hugovk commented Aug 22, 2019

Same as #68 but made from a non-fork branch.

cc @cclauss.

@ghost
Copy link

ghost commented Aug 22, 2019

DeepCode Report (#0078ac)

DeepCode analyzed this pull request.
There are no new issues.

@codecov
Copy link

codecov bot commented Aug 22, 2019

Codecov Report

Merging #70 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #70   +/-   ##
=======================================
  Coverage   98.96%   98.96%           
=======================================
  Files           2        2           
  Lines         290      290           
=======================================
  Hits          287      287           
  Misses          3        3
Impacted Files Coverage Δ
src/pypistats/__init__.py 99.03% <ø> (ø) ⬆️
src/pypistats/cli.py 98.79% <100%> (ø) ⬆️

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 65349de...0078ac8. Read the comment docs.

@hugovk
Copy link
Owner Author

hugovk commented Aug 22, 2019

Thanks again for the PRs, this is looking good. I've replicated what Azure Pipelines is doing, with the exception of uploading coverage to Codecov, and the sdist check. They can wait for later.

GitHub Actions looks very good. This is only using it as CI, and it's very like AP. Improvements:


https://help.github.com/en/articles/events-that-trigger-workflows#webhook-events says of push:

Triggered on a push to a repository branch. Branch pushes and repository tag pushes also trigger webhook push events.

And pull_request:

Triggered when a pull request is assigned, unassigned, labeled, unlabeled, opened, edited, closed, reopened, synchronize, ready_for_review, locked, unlocked or when a pull request review is requested or removed.

What a lot of events!

https://help.github.com/en/articles/events-that-trigger-workflows#pull-request-events-for-forked-repositories says:

When you open a pull request from a forked repository, the pull request opens on the base repository by default. In this scenario, GitHub sends the pull_request event to the base repository and no pull request events occur on the forked repository because the webhook configuration of the original base repository doesn't apply to forked repositories.

Does that mean pull_request is needed to trigger builds from forks? It continues:

If you intend to use the pull_request event to trigger CI tests, we recommend that you set up your workflow configuration to listen to the push event.

Is that saying to use both push and pull_request ? I guess so. Will send beta feedback.

It also means for each event, there's 2 builds. 2 × (3 OS × 2 Python tests + 1 OS × 1 Python lint) = 2 × 7 = 14.

Still within the 20 limit here, but AP doesn't double up.


Open questions

@hugovk hugovk added the enhancement New feature or request label Aug 23, 2019
@cclauss
Copy link
Contributor

cclauss commented Aug 23, 2019

@chrispat Any comments or recommendations on this exploration?

@hugovk hugovk merged commit d278aa1 into master Aug 31, 2019
@hugovk hugovk deleted the cclauss-patch-2 branch August 31, 2019 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants