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

Create github actions #200

Merged
merged 11 commits into from
Jan 12, 2021
Merged

Create github actions #200

merged 11 commits into from
Jan 12, 2021

Conversation

kmacdonald-stsci
Copy link
Collaborator

Created github actions workflow to perform pytests on two python versions, as well as testing a document build.

@pllim
Copy link
Contributor

pllim commented Jan 5, 2021

Please squash out blank.yml and delete .travis.yml.

run: |
pytest

doc_build:
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use RTD PR builder?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I removed .travis.yml. I renamed the github action blank.yml to spherical_geometry.yml.

@pllim
Copy link
Contributor

pllim commented Jan 5, 2021

Also add a badge to README.

@nden
Copy link
Contributor

nden commented Jan 5, 2021

AFAIT there's a consistent issue with the RTD doc builder. I suspect it needs higher permissions to sync the webhook than any of us has. I'll contact someone else about this but meanwhile I think this is OK to go without it.

@pllim
Copy link
Contributor

pllim commented Jan 5, 2021

Huh! I didn't have that problem with my own spacetelescope repos. I wonder what changed since I set it up.

@nden nden closed this Jan 8, 2021
@nden nden reopened this Jan 8, 2021
@nden nden requested a review from jdavies-st January 8, 2021 23:03
.github/workflows/spherical_geometry.yml Outdated Show resolved Hide resolved
.github/workflows/spherical_geometry.yml Outdated Show resolved Hide resolved
@jdavies-st
Copy link
Contributor

@mcara I see that the above check for continuous-integration/travis-ci seems to be required for PRs to be merged. Now that Travis CI is being removed, this should be changed in the settings.

Copy link
Contributor

@jdavies-st jdavies-st left a comment

Choose a reason for hiding this comment

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

Looks good!

At some point in the future if we switch to using tox here, it might be good to run tests on the installed version of the code, especially since this has a bunch of C code. But that's for a future task I think.

@pllim
Copy link
Contributor

pllim commented Jan 11, 2021

I dunno about tox. I am struggling to get it to find my C-ext over at synphot. No one could figure out why... spacetelescope/synphot_refactor#295

I kinda regret switching to it.

@mcara
Copy link
Member

mcara commented Jan 11, 2021

@mcara I see that the above check for continuous-integration/travis-ci seems to be required for PRs to be merged. Now that Travis CI is being removed, this should be changed in the settings.

How should I do this? I removed travis from webhooks but it did not have any effect. @pllim can you advise?

Maybe simply merging this PR will remove travis requirement?

@codecov
Copy link

codecov bot commented Jan 11, 2021

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #200   +/-   ##
=========================================
  Coverage          ?   65.75%           
=========================================
  Files             ?        8           
  Lines             ?     1244           
  Branches          ?        0           
=========================================
  Hits              ?      818           
  Misses            ?      426           
  Partials          ?        0           

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 a2e266b...d893c50. Read the comment docs.

@pllim
Copy link
Contributor

pllim commented Jan 11, 2021

continuous-integration/travis-ci

It is under Settings -> Branches (branch protection rules). I updated it to require subset of Actions CI and not Travis CI anymore.

.github/workflows/spherical_geometry.yml Outdated Show resolved Hide resolved
.github/workflows/spherical_geometry.yml Outdated Show resolved Hide resolved
.github/workflows/spherical_geometry.yml Show resolved Hide resolved
@jdavies-st
Copy link
Contributor

Looks great! I think this is ready for merging @mcara .

@mcara mcara merged commit 52f3b6f into spacetelescope:master Jan 12, 2021
@kmacdonald-stsci kmacdonald-stsci deleted the create_ga branch January 16, 2021 00:08
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.

5 participants