search

Shopify / shipit-engine

Deployment coordination
https://shopify.engineering/introducing-shipit
MIT License
1.41k stars 144 forks source link

Upgrade Rails to 7.1.1 #1324

Closed kartiki975 closed 7 months ago

kartiki975 commented 7 months ago
kartiki975 commented 7 months ago

@casperisfine Post rails upgrade, mini_racer is breaking wherever Ruby version is less than 3.2 in the Github Actions. Looking at https://github.com/rubyjs/mini_racer?tab=readme-ov-file#supported-ruby-versions--troubleshooting, it seems like there is no workaround where the Ruby version is less than 3.2:

make sure you have Rubygems >= 3.2.13 and bundler >= 2.2.13 installed: gem update --system

Note, I have tried this with the existing and now, the latest version of mini_racer here: ba2c64a.

At this point, there two actions I can take:

Thoughts?

casperisfine commented 7 months ago
  • removing mini_racer; do you know if this will break other changes?

We can remove mini_racer and if nodejs is around, ExecJS will automatically use tha, it's probably better that way.

casperisfine commented 7 months ago

Thanks for taking care of the upgrade, LGTM.

However next time please try to keep a clean git history, wip and Add minor tweaks commit messages cause pain down the line when investigating the reason for a change.

benlangfeld commented 7 months ago

Thanks for taking care of the upgrade, LGTM.

However next time please try to keep a clean git history, wip and Add minor tweaks commit messages cause pain down the line when investigating the reason for a change.

Worth setting the repo to force squash merges? Works wonders at my shop.

casperisfine commented 7 months ago

Worth setting the repo to force squash merges?

It's one solution, but has its own downsides as some PR do legitimately contain more than one commit, e.g. a refactoring followed by a feature, etc.

Anyway, this was a general advice for contribution to all projects, not specific to shipit-engine.