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 from travis to Github actions #27
Conversation
860169e
to
de8de8a
Compare
c0d1b5d
to
f872037
Compare
now done via github actions
f872037
to
b338d8f
Compare
run: bundle exec rake spec | ||
|
||
- name: Coveralls Parallel | ||
uses: coverallsapp/github-action@master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The best practice I've seen recommended is to pin a specific version/SHA and use dependabot for upgrading the pinned dependency. Not sure how we want to do it here, though -- this potentially has an impact on the action we generate as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's a relatively simple command and maintained by the coveralls devs. yeah ideally they should have an @v1 branch but alas they do not. My concern of locking it to a specific hash is if they change something for their infrastructure it'll break anyways..
Anyways.. @ashkulz re-review this branch again though as I finally got "everything" working with GH actions (including all the DB testing used by the other gems)
Next-up is adding in rails 7.0 (since they have RC1 out)
5c65145
to
737b0a6
Compare
- cleanup config to handle DB versions better - rewrite the "create_databases/drop_databases" to utilize AR's database tasks - update initial gem template
737b0a6
to
75a9c5c
Compare
name: 'Run bundle update', | ||
run: 'bundle update', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we want to do a bundle update
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to ensure we are using the latest versions of the packages as everything is being "cached" from the above ruby install above. (though I guess I do need to test to see if it is actually caching the lock file)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not simply disable the cache then? I don't think we depend on a lot of gems, so won't add too much overhead (especially since we're launching database instances via docker).
@urkle I'll have to see how the generated github action configuration looks like, may take me a day or two as a lot going on IRL. |
@ashkulz thanks, yeah there is a bit there.. But all those "variants" have been tested so if you look through the history of GH actions you can see each of the variations being tested up there. I thought about instead creating a custom GH Action that I could use to wrap up that logic (so the ones for the gems are simpler) but I just want to get this moving for now so we can get to testing AR 6+ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pull_request: nil, | ||
}, | ||
concurrency: { | ||
group: '${{ github.head_ref }}', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the generated YAML, it comes out quoted for some reason -- see this failing run. If you look at this PR, it works fine when unquoted -- not sure how we can force that? We might need to migrate to a text-template like approach 🤷♂️
end | ||
end | ||
end | ||
|
||
desc 'Create database for CI run' | ||
task :create_ci_database do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to merge this earlier, as I'm not sure how to get a git dependency installed during CI -- this causes an error.
--health-interval 5s \\ | ||
--health-timeout 5s \\ | ||
--health-retries 5 \\ | ||
--name database mysql:5.6 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason why we're not supporting dbversions here? I think there's 5.6, 5.7 and 8.0 tags available at dockerhub.
No description provided.