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

fix(travis): remove travis CI, replace with GH Actions #400

Merged
merged 34 commits into from
Sep 15, 2023
Merged

Conversation

Avantol13
Copy link
Contributor

@Avantol13 Avantol13 commented Sep 12, 2023

New Features

Breaking Changes

Bug Fixes

Improvements

  • fix(travis): remove travis CI, replace with GH Actions

Dependency updates

Deployment changes

@github-actions
Copy link

The style in this PR agrees with black. ✔️

This formatting comment was generated automatically by a script in uc-cdis/wool.

@Avantol13 Avantol13 changed the title fix(travis): force a reasonable default distribution fix(travis): remove travis CI, replace with GH Actions Sep 14, 2023
jawadqur
jawadqur previously approved these changes Sep 15, 2023
Comment on lines 6 to 11
# since this whole thing is run as a bash {{this script}}, only the last pytest
# command controls the exit code. We actually want to exit early if something fails,
# so check that here and exit as necessary
RESULT=$?
if [ $RESULT -ne 0 ]; then
exit 1
Copy link
Contributor

Choose a reason for hiding this comment

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

would it work to just set -e at the beginning of the script?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done and I tested it works by purposing failing a test in the middle run 👍

Comment on lines +24 to +25
setup-script: 'tests/ci_setup.sh'
test-script: 'tests/ci_commands_script.sh'
Copy link
Contributor

Choose a reason for hiding this comment

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

since these 2 values are the same as the defaults, you could remove them from here, unless you want it to be more explicit where the files are used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I figured since: a) I was gonna use this as an example PR for others to base theirs and b) it's clearer how it's working this way, it'd be better to be explicit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants