Skip to content
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.

Sign up for GitHub

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

Generate Gemfile entry with version constraint #1244

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Generate Gemfile entry with version constraint #1244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/install/web.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def apply_template!
if ARGV.include?("--suspenders-main")
gem "suspenders", github: "thoughtbot/suspenders", branch: "main"
else
gem "suspenders"
gem "suspenders", "> 20240101"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I tried (and failed) to see if I could do this dynamically by including Suspenders::Version in this file, and then doing something like this:

Suggested change
gem "suspenders", "> 20240101"
gem "suspenders", "> #{Suspenders::VERSION}"

The problem is that when this file is invoked, it's outside the context of Suspenders, so if Suspenders is not installed on the system, it will error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, this is a Ruby file executed in its own environment without access to Bundler or other dependencies. I chose 20240101 because its a lower limit on the possible values (versions starting with the 2024-prefix), not necessarily because it's a valid version that has a release.

Copy link
Contributor

Choose a reason for hiding this comment

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

this is a Ruby file executed in its own environment without access to Bundler or other dependencies

I wonder if we could do something similar to the Rails reproduction scripts, but for now, this works.

Copy link
Contributor

Choose a reason for hiding this comment

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

As an aside, I thought leaving off the version meant we'd download the latest version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wonder if we could do something similar to the Rails reproduction scripts

While that might increase the runtime of rails new, that might be worthwhile, since it's (ideally) only executed once per project.

As an aside, I thought leaving off the version meant we'd download the latest version?

Ideally, that would be true. In practice, rails new would consistently install v0.2.4.

end
end

Expand Down
8 changes: 8 additions & 0 deletions test/generators/suspenders/install/web_generator_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@ class WebGeneratorTest < Rails::Generators::TestCase
setup :prepare_destination
teardown :restore_destination

test "generates Gemfile entry with version constraint" do
with_database "postgresql" do
run_generator
end

assert_file "Gemfile", /gem "suspenders", "> 20240101"/
end
Comment on lines +15 to +21
Copy link
Contributor

Choose a reason for hiding this comment

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

Were you able to get this to pass locally? I'm having some local dependency issues, so I can't verify this locally.

That being said, the file we're modifying in this commit is lib/install/web.rb, which is not a generator, but the application template.

This test is responsible for testing Suspenders ::Generators::Install:: WebGenerator, which doesn't install Suspenders.

Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately, we don't have any coverage for lib/install/web.rb.


test "raises if API only application" do
within_api_only_app do
assert_raises Suspenders::Generators::APIAppUnsupported::Error do
Expand Down
Loading