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

Update CI config #83

Merged
merged 2 commits into from Feb 2, 2020
Merged

Update CI config #83

merged 2 commits into from Feb 2, 2020

Conversation

hugovk
Copy link
Collaborator

@hugovk hugovk commented Feb 2, 2020

  • There's no need for the "drop the dot" workaround, it will use the interpreter tox is installed to
  • Also test on Python 3.8

@codecov
Copy link

codecov bot commented Feb 2, 2020

Codecov Report

Merging #83 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #83   +/-   ##
=======================================
  Coverage   70.37%   70.37%           
=======================================
  Files           5        5           
  Lines         297      297           
  Branches       35       35           
=======================================
  Hits          209      209           
  Misses         88       88

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 df6400f...fc68f1b. Read the comment docs.

Copy link
Owner

@ofek ofek left a comment

Choose a reason for hiding this comment

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

Nice! Mind if we change on: [push, pull_request] to on: [push] to remove double builds on PRs? See #84

Would that fix it?

@hugovk
Copy link
Collaborator Author

hugovk commented Feb 2, 2020

I think that would mean we wouldn't get builds from forks.

For example, this PR was submitted from my fork, and there's only pull_requests shown.

Another example: here's a PR in another repo that also has on: [push, pull_request], from someone who's not a project member: python-pillow/Pillow#4398. There's only pull_request builds shown (click the green tick next to the commit message).

@ofek
Copy link
Owner

ofek commented Feb 2, 2020

Ah here you go, I found the magic 🙂 #85

@hugovk
Copy link
Collaborator Author

hugovk commented Feb 2, 2020

From #85:

on:
  push:
    branches:
    - master
  pull_request:
    branches:
    - master

Will that prevent PRs building from feature branches? And from forks?

@ofek
Copy link
Owner

ofek commented Feb 2, 2020

Forks no (I think, try it here!) but feature branches on this repo that have not opened PRs yet yes (I expect no CI until PR so that's good imo)

@hugovk
Copy link
Collaborator Author

hugovk commented Feb 2, 2020

Usually, the recommended workflow for contributors is to fork the repo, create a feature branch, and then a PR.

So I think it's very important to test against forks and feature branches.

The tests are so quick to run (between 22s - 1m), and we have up to 20 jobs in parallel, I'd be inclined to leave it to test more rather than less.

@ofek
Copy link
Owner

ofek commented Feb 2, 2020

Fair point, though I think it's atypical for contributors (or anyone really) to look at CI without opening a PR. I know I never do 🙂

Feel free to merge whenever

@hugovk hugovk merged commit 502b478 into ofek:master Feb 2, 2020
@hugovk hugovk deleted the simplify-ci-config branch February 2, 2020 21:48
REestwick pushed a commit to REestwick/pypinfo that referenced this pull request Apr 1, 2024
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.

None yet

2 participants