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 build actions #107

Merged
merged 1 commit into from Oct 18, 2019
Merged

Add github build actions #107

merged 1 commit into from Oct 18, 2019

Conversation

amyreese
Copy link
Contributor

@amyreese amyreese commented Aug 26, 2019

Will build on 3.6 and 3.7, on Linux and Mac. Windows builds will be added once it is fully supported in Bowler, because Github Actions don't currently support the notion of jobs that are allowed to fail. Also drops the Travis CI config and switches to Codecov instead of Coveralls for test coverage reports.

@amyreese amyreese requested a review from thatch August 26, 2019 23:35
@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Aug 26, 2019
Copy link
Contributor

@thatch thatch left a comment

Choose a reason for hiding this comment

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

Overall, it's fine. I see one glitch in the step names, but I think it's an upstream bug.

@@ -1,19 +1,19 @@
venv:
python3 -m venv .venv
python -m venv .venv
Copy link
Contributor

Choose a reason for hiding this comment

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

PYTHON ?= python at the top

then

$(PYTHON) elsewhere

python3 -m pip install -Ur requirements.txt
python3 -m pip install -Ur requirements-dev.txt
python -m pip install -Ur requirements.txt
python -m pip install -Ur requirements-dev.txt

dev: venv
source .venv/bin/activate && make setup
Copy link
Contributor

Choose a reason for hiding this comment

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

PYTHON=.venv/bin/python make setup
.venv/bin/python setup.py develop

?

- name: Coverage
continue-on-error: true
run: |
coveralls
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this going to update the same location we had with travis?

run: |
python -m pip install --upgrade pip
make setup
pip install -U .
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't need the install step if we use PYTHON=.venv/bin/python make test, etc... up to you

Copy link
Contributor

Choose a reason for hiding this comment

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

I played with this last night, venv on Windows puts the binaries in a different spot. What you have is fine.

Will build on 3.6 and 3.7, on Linux and Mac. Updates makefile to use
`python` instead of `python3` because Windows is weird. Windows builds
will come later when better support for spawned processes is added.
@codecov-io
Copy link

codecov-io commented Oct 17, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@c10f801). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #107   +/-   ##
=========================================
  Coverage          ?   79.15%           
=========================================
  Files             ?       12           
  Lines             ?      801           
  Branches          ?       77           
=========================================
  Hits              ?      634           
  Misses            ?      143           
  Partials          ?       24

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 c10f801...f894e32. Read the comment docs.

@amyreese amyreese merged commit 1d77592 into master Oct 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants