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

Switch to Github Actions #402

Merged
merged 14 commits into from
Jan 1, 2020
Merged

Switch to Github Actions #402

merged 14 commits into from
Jan 1, 2020

Conversation

kavigupta
Copy link
Contributor

@kavigupta kavigupta commented Dec 28, 2019

They are a bit faster and significantly more stable

Copy link
Contributor

@mehrdadn mehrdadn left a comment

Choose a reason for hiding this comment

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

@kavigupta I switched from Travis to GitHub Actions; it seems to have fewer headaches to deal with.
Can you verify that it's testing everything it's supposed to (and otherwise doing fine), and if so, delete the merge requirement for Travis (and add one for GitHub Actions instead)?

@kavigupta
Copy link
Contributor Author

kavigupta commented Dec 28, 2019

You're going to need to get @Sumukh or @colinschoen to lgtm, I don't feel entirely comfortable making this global of a change without approval

.github/workflows/main.yml Outdated Show resolved Hide resolved
@kavigupta
Copy link
Contributor Author

This seems quite a bit faster and a bit cleaner! The one thing I think it's missing is the slack integration but I'm not confident that even worked before since I don't remember getting any slack notifications about okpy

@kavigupta
Copy link
Contributor Author

kavigupta commented Dec 29, 2019

Wait why are there 12 tests? are they randomly placed either on pull_request or branch?

Actually, wait there's 16 tests? How does that add up?

@mehrdadn
Copy link
Contributor

Yeah I took out the Slack integration because it seemed to be already useless.
I think the difference between push and pull_request builds is that one is pre-merge and one is post-merge. Each one occurs for every possibility in the matrix. Sadly I had a hard time figuring out how to get them labeled better... the only way I could think of was to list each matrix entry manually and assign a name to it myself, but that seemed ugly.

@kavigupta
Copy link
Contributor Author

yeah the push and pull_request seem the same as travis's branch and pull request, which is nice

Labelling is probably unnecessary, if there's an error we can just click through

Linux seems to be missing, which is fine I guess (it's pretty covered by osx and not actually used much) but it would be ideal if we had it

@mehrdadn
Copy link
Contributor

It's easy to add, I can add it.

@kavigupta
Copy link
Contributor Author

It's easy to add, I can add it.

thanks!

@mehrdadn mehrdadn force-pushed the fix-windows-tests branch 7 times, most recently from bedcee6 to a094646 Compare December 29, 2019 02:20
@mehrdadn
Copy link
Contributor

@kavigupta ok done. I also named them explicitly since it was kind of annoying to discover the versions otherwise.

@kavigupta kavigupta changed the title Update windows install scripts Switch to Github Actions Jan 1, 2020
@kavigupta kavigupta merged commit 837717b into master Jan 1, 2020
@kavigupta kavigupta deleted the fix-windows-tests branch January 1, 2020 21:11
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.

2 participants