rails-template
rails-template copied to clipboard
Fixes for Github Actions
This changeset resolves issues I had with Github Actions not passing on a new Rails application with all the trimmings:
- Actions started a DB container, but the new version of database.yml doesn't pick up the env vars to connect to the database. I'm happy to bikeshed what we name the env vars since the current naming scheme is not compatible with the
libpq
defaults - Actions was running
rails db:setup
, which is not representative of the complete set of steps that an app might need to run tests. In particular, Sidekiq provisions SIDEKIQ_WEB_USERNAME which is expected to be present. I think the least surprising thing is for actions to run bin/setup to copy environment variables, migrate & seed the database, and anything else the app might consider 'setup'.
I'm happy to bikeshed what we name the env vars since the current naming scheme is not compatible with the libpq defaults
Maybe I'm missing something, but is it possible for us to use the defaults? That would be my preference.
I think the least surprising thing is for actions to run bin/setup
So this was a conscious decision in the original PR - I think we need to discuss further and decide properly (I personally have not been happy that we're installing overcommit
in CI, but that can be addressed in an alternative way if we decide to switch back to running bin/setup
).
I created an app with sidekiq & devise and the CI is indeed broken because of ENV vars but I haven't had any issues with the DB env vars
I have a WIP PR at https://github.com/eoinkelly/temp-rails-tpl-github-actions-demo/pull/1/files to get the demo to a working state.
@G-Rath We could test for presence of a CI
and skip the overcommit stuff in bin/setup
.
CI=true
seems to have good support:
Of course, that doesn't really address it's potential evolution to included slow steps. I know that was my original point in the referenced issue but upon reflection, we almost never change bin/setup
so maybe it's not a big deal?
Looked into this. We would need to skip overcommit and seeds. bin/setup
also runs db:create:all
and then migrate
which is a slower than db:setup
(which uses db:schema:load
instead of db:migrate
).
I'm now in favour of sticking with what we currently have.
https://github.com/ackama/rails-template/blob/main/bin/setup#L3-L18
The build is actually working now, so merging this PR doesn't really add anything. We've discussed this in Ruby Guild and determined this doesn't really add any value given the build is not broken