solidusio / solidus_dev_support

A collection of tools for developing Solidus extensions.
MIT License
21 stars 27 forks source link

Loosen puma and rspec-rails version constraints in solidus_dev_support #178

Closed gsmendoza closed 3 years ago

gsmendoza commented 3 years ago

Goal

As a solidus_starter_frontend user

I would like solidus_dev_support to support puma ~> 5.0

And I would like solidus_dev_support to support rspec-rails ~> 5.0

So that if solidus_starter_frontend is installed with specs on a Rails app with those versions of the gems, solidus_starter_frontend won't have to revert the gems to older version in order to support solidus_dev_support.

Background

In https://github.com/nebulab/solidus_starter_frontend/pull/175/files#r690067361, SolidusStarterFrontend::RspecGenerator#install has to revert the puma version of a Rails 6.1.4 app to 4.x. Otherwise, it won't be able to install solidus_dev_support. Rails 6.1.4 sets puma to '~> 5.0'.

In https://github.com/nebulab/solidus_starter_frontend/pull/180, the generator also has to revert rspec-rails to ~> 4.0.0.beta3 in order to install solidus_dev_support.

Issue demonstration

https://www.loom.com/share/b4f5f9c40bac4b9fad48612e5e4794e0

In this video, I demonstrate how SolidusStarterFrontend::RspecGenerator#install forces the puma version of its client app to 4.3 in order to accomodate the version constraint of puma in SolidusDevSupport.

Fix demonstration

https://www.loom.com/share/1d8a6360bb68471ea75692298b606c13

In this video, I demonstrate how loosening the version constraint of puma fixes the issue with SolidusStarterFrontend::RspecGenerator#install. The demonstration points to the following SolidusStarterFrontend branch: https://github.com/nebulab/solidus_starter_frontend/tree/gsmendoza/eng-211-loosen-puma-version-constraint-in-fix-demonstration.

Checklist

mergify[bot] commented 3 years ago

It looks like this PR is missing a label to determine the type of change it introduces. The maintainer should add one of the following labels:

Additionally, the maintainer may also want to add one of the following:

Once the correct labels have been set, simply remove the needs changelog label label from this PR so I can merge it.

waiting-for-dev commented 3 years ago

Wouldn't it be better to use something like ~> 5.4? >= 4.3 means that this version of solidus_dev_support will be compatible with all future versions of puma, including those introducing breaking changes on major version number bumps (like 6.0), which is something we can't assure. If we still want to support 4.3, I think we could use something like gem 'puma', '>= 4.3', '< 6.0'

gsmendoza commented 3 years ago

@waiting-for-dev I saw your comment only now. I like the approach you proposed of setting the version constraint within a certain range e.g. gem 'puma', '>= 4.3', '< 6.0'. I think this is the most conservative way we can update the constraints. I'll update the PR. Done.