-
Notifications
You must be signed in to change notification settings - Fork 54
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 Actions for CI #2794
Conversation
success with the JS, and it's faster than Travis to start. going to trying incrementally adding Ruby, which is more complicated because of Postgres, and then add caching and hopefully 🚢 49 44 AM" src="https://user-images.githubusercontent.com/1056957/77775690-47dc2f00-7023-11ea-8b71-c8e47e7de602.png"> |
parallel jobs works as expected, and first set of Ruby steps look great 👍 |
…n iterate more quickly to get this working
This reverts commit d573b63.
For posterity, what took forever with all these commits is that Rails wasn't reading |
selfie |
We're having issues with Travis and still haven't been able to resolve them with their support team, so let's see what it takes to run tests with GitHub actions. This aims to just run the JS tests.
EDIT: Iterated on this to get everything working. Overall, the upside here is to remove Travis as a provider, which is a new benefit since after last week, when CI went down for a few days and we support couldn't help with even agreeing on whether it was up or not. Based on the assumption that GitHub will have better stability (and investment and growth) going forward from here, this seems like an improvement. We can remove the related permission grants to Travis as well, which were fairly permissive.
This also speeds up build time in both JS and in Ruby, and in particular so far the "time to spin up the build worker" has been close to zero. With Travis, there have been several times where the time to spin up the worker has been significant, so this migration leaves CI faster overall. I separately looked very briefly at parallelizing rspec but seems this still hasn't landed as first class yet (there are several ways to work around, but I didn't spend any time on it as this is already a large change and we already get improved test times). Here's what this PR sees:
That's 3:45 for JS (compared to 4:24 in Travis) and 10:30 for Ruby (compared to 13:34 in Travis). 👍 😄 🕐
I think the only thing we lose is the nice coloring in the build log, and there is some increased latency to see the output of a build step (eg, to see logs in rspec tests while they are running, for debugging drift between local tests/CI). These seem minor compared to the speedups, but this is what is lost:
Finally, there is some additional deployment risk since this changes what looked like a spacing typo in the
database.yml
config for production. I fixed that spacing and deleted all the commented out bits that are irrelevant to make that kind of problem more visible. This code bug has been present since #2540 but I also verified that it hasn't mattered andsslmode
is set to require in all production environments because of thePGSSLMODE
env variable.So this is ready to 🚢 and then we can remove Travis and related permissions.