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

Move CI to GitHub actions #116

Merged

Conversation

bernardopericacho
Copy link
Contributor

Proposed changes:

@mhe @addisonElliott we will need to create another job or workflow for publishing the releases before we remove the travis.yml file completely

@bernardopericacho
Copy link
Contributor Author

This has been tested and it is working in my fork: https://github.com/bernardopericacho/pynrrd/actions/runs/2304161140

jobs:
test:
name: test
runs-on: ubuntu-18.04
Copy link
Contributor Author

Choose a reason for hiding this comment

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

using ubuntu-18.04 instead of ubuntu-latest so we can run tests over Python 3.4. This is a pure migration of what we already have in travis but we can discuss further if we should still be supporting versions that are deprecated (< 3.7)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Great point. I'm fine dropping Python 3.4 support in testing. It's been EOL for quite awhile now.

Let's change to ubuntu-latest and use a newer Python version. Maybe 3.7 and 3.9 for testing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that sounds good to me, should I also drop support for 3.4, 3.5 and 3.6 in the setup.py as part of this PR?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, that's probably a good plan.

@addisonElliott addisonElliott self-requested a review May 11, 2022 00:56
@addisonElliott
Copy link
Collaborator

addisonElliott commented May 11, 2022

🎉 This is wonderful!

I have no objection to GitHub Actions. Seems to be the future now-a-days.

As it stands now, the Travis deployment script doesn't work. I've been deploying manually via CLI, which is fine since it's not done often. I'm all for getting that back up and running via GitHub actions.

For this PR, let's leave out the deployment part. We can address that in a separate PR. It should be pretty simple to setup and then @mhe will need to add a secret to the repository I believe.

jobs:
test:
name: test
runs-on: ubuntu-18.04
Copy link
Collaborator

Choose a reason for hiding this comment

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

Great point. I'm fine dropping Python 3.4 support in testing. It's been EOL for quite awhile now.

Let's change to ubuntu-latest and use a newer Python version. Maybe 3.7 and 3.9 for testing?

Copy link
Contributor Author

@bernardopericacho bernardopericacho left a comment

Choose a reason for hiding this comment

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

thanks for the quick review!

jobs:
test:
name: test
runs-on: ubuntu-18.04
Copy link
Contributor Author

Choose a reason for hiding this comment

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

that sounds good to me, should I also drop support for 3.4, 3.5 and 3.6 in the setup.py as part of this PR?

@mhe
Copy link
Owner

mhe commented May 11, 2022

That's awesome, thanks! I'm all for it!

Copy link
Contributor Author

@bernardopericacho bernardopericacho left a comment

Choose a reason for hiding this comment

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

Feedback addressed

setup.py Show resolved Hide resolved
@addisonElliott addisonElliott merged commit 7213262 into mhe:master May 11, 2022
@bernardopericacho bernardopericacho deleted the move_testing_to_gh_actions branch June 5, 2022 20:21
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.

3 participants