Skip to content

Consolidate how Rails version is fetched for dummy-app and sandbox#202

Merged
waiting-for-dev merged 1 commit into
masterfrom
waiting-for-dev/fix_rails_version
Mar 3, 2023
Merged

Consolidate how Rails version is fetched for dummy-app and sandbox#202
waiting-for-dev merged 1 commit into
masterfrom
waiting-for-dev/fix_rails_version

Conversation

@waiting-for-dev

Copy link
Copy Markdown
Contributor

Summary

We use now the same that it's used for the sandbox.

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

The following are not always needed:

  • 📖 I have updated the README to account for my changes.
  • 📑 I have documented new code with YARD.
  • 🛣️ I have opened a PR to update the guides.
  • ✅ I have added automated tests to cover my changes.
  • 📸 I have attached screenshots to demo visual changes.

@waiting-for-dev waiting-for-dev requested a review from elia March 2, 2023 11:47
Comment thread bin/dummy-app
rm -rf ./dummy-app
rails ${RAILS_VERSION:+_${RAILS_VERSION}_} new dummy-app \
rails_version=`bundle exec ruby -e'require "rails"; puts Rails.version'`
rails _${rails_version}_ new dummy-app \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've had issues with using this setup calling bundle exec, inside the script, thoughts on doing it the other way around and updating the sandbox instead?

Using the env var seems more adaptable to different situations and the Gemfile would be based on an environment variable anyway.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is that in the script context, $RAILS_VERSION is expected to be an actual version, like 7.0 or 6.2. However, in other places of the Solidus ecosystem is expected to be a semver reference, like ~> 7.0. See docker config on main Solidus or the CircleCI orb as examples. (I use docker for development, and my docker-compose file had the env var; that was how it hit me 😅).

Do you recall which problem you found with bundle exec? At first sight, it looks like the safest way to get the actual version that we're using.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No I think it was related to versions of bundler one nested into the other, but I don't have any reference, let's go with this code and if the problem comes up again we can switch to another solution 👍

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All right, thanks, @elia!

@elia elia left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙌

@waiting-for-dev waiting-for-dev merged commit 9ea732b into master Mar 3, 2023
@waiting-for-dev waiting-for-dev deleted the waiting-for-dev/fix_rails_version branch March 3, 2023 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants