Conversation
.github/workflows/ci.yml
Outdated
| with: | ||
| path: vendor/bundle | ||
| key: gems-build-rails-${{ matrix.rails_version }}-ruby-${{ matrix.ruby_version }}-${{ hashFiles('**/Gemfile.lock') }} | ||
| key: gems-build-rails-${{ matrix.rails_version }}-ruby-${{ matrix.ruby_version }}-${{ hashFiles('**/gemfiles/rails_${{ matrix.rails_version }}.gemfile.lock') }} |
There was a problem hiding this comment.
I'm not sure this works as expected. 🤔
There was a problem hiding this comment.
I think this part probably works alright. I'm inclined to believe it's setting BUNDLE_GEMFILE that isn't doing anything.
if caching was failing but the Gemfile was fine, GitHub Actions would try to install turbo-rails. I'm inclined to believe setting the Gemfile doesn't work, but caching still does, because there's no mention of turbo-rails or tailwindcss-rails in the logs, and the logs still say they're using a gem that has already been downloaded.
There was a problem hiding this comment.
tailwindcss-rails now appear in the logs of the latest runs.
There was a problem hiding this comment.
It doesn't look like the call to hashFiles is returning anything.
|
Might it make sense to use Appraisal directly the same way it's mentioned in |
Something like that? - bundle exec rake
+ bundle exec appraisal rails-${{ matrix.rails_version }} rake |
|
Yeah. Come to think of it, I don't think it would cause caching issues, since it'd be installing the gems to the same place. You're right to be setting the cache keys based on the Rails version in use, though. |
|
What do you think of this approach @joelhawksley & @BlakeWilliams? |
59f6c25 to
bf847cf
Compare
|
I finally got CI to pass, with appraisal running the specs. It's currently running a fork of Appraisal, with this PR thoughtbot/appraisal#174 merged. So we should probably wait for the new version of Appraisal before going forward with this. |
| gem "rails", rails_version == "main" ? { git: "https://github.com/rails/rails", ref: "main" } : rails_version | ||
|
|
||
| group :development, :test do | ||
| gem "appraisal", github: "excid3/appraisal", branch: "fix-bundle-env" |
There was a problem hiding this comment.
Is there a reason we shouldn't be using thoughtbot/appraisal?
There was a problem hiding this comment.
Yes because the fix we need is not yet merged: thoughtbot/appraisal#174
There was a problem hiding this comment.
Is there a way we can keep tabs on where @excid3's fork is at to ensure it's not susceptible to upstream issues with Appraisal?
There was a problem hiding this comment.
Just FYI, I am skipping appraisal in CI right now and just setting BUNDLE_GEMFILE via matrix.
https://github.com/excid3/noticed/blob/master/.github/workflows/ci.yml#L28
boardfish
left a comment
There was a problem hiding this comment.
The call to hashFiles when setting the cache key isn't returning anything, so the cache can be overwritten by PRs.
.github/workflows/ci.yml
Outdated
| with: | ||
| path: vendor/bundle | ||
| key: gems-build-rails-${{ matrix.rails_version }}-ruby-${{ matrix.ruby_version }}-${{ hashFiles('**/Gemfile.lock') }} | ||
| key: gems-build-rails-${{ matrix.rails_version }}-ruby-${{ matrix.ruby_version }}-${{ hashFiles('**/gemfiles/rails_${{ matrix.rails_version }}.gemfile.lock') }} |
There was a problem hiding this comment.
It doesn't look like the call to hashFiles is returning anything.
Hmmm... I think that we cannot hash the |
|
I think it might be sufficient to use the Gemfiles themselves under |
|
Seems like the multiple calls to the |
|
What needs doing to move this forward? Is there any way I can help? From what I can see it's probably just the gem caching that remains to be solved, but I'm not sure if you had any other plans @Spone. |
Yes I think the last blocking item is fixing the caching issue. If you have time to look into it that would be awesome! |
|
I'll close this, as it's superseeded by #1308. |
Related to #1227 (comment)
In my initial setup of Appraisal (#1225), I forgot to specify which gemfile to use in the CI configuration.