allow upgrade to rails 7

bobf commented 2 years ago

@dorianmariefr Thanks for the PR ! I have a couple of requests:

Could you change the version constraint for Rails to ~> 6 ? This will allow any version of Rails 6, 7, 8, etc. but not Rails 5. i.e.:
```
spec.add_runtime_dependency 'rails', '~> 6'
```
Or, if the < 7.1 constraint is needed (not sure I understand this ?) then:
```
spec.add_runtime_dependency 'rails', ['~> 6', '<  7.1']
```
Could you add rails to the ignore: section of a new file called .strong_versions.yml, e.g.:
```
# .strong_versions.yml
ignore:
- rails
```
Then ensure that make test passes.

Just a couple of quick changes and then I'll happily accept your PR.

Cheers,

dorianmariecom commented 2 years ago

thanks, make test passes

dorianmariecom commented 2 years ago

Cool

On Wed, Dec 15 2021 at 3:09 PM, bobf < @.*** > wrote:

Merged #7 ( https://github.com/bobf/action_mailbox_amazon_ingress/pull/7 ) into master.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub ( https://github.com/bobf/action_mailbox_amazon_ingress/pull/7#event-5771618831 ) , or unsubscribe ( https://github.com/notifications/unsubscribe-auth/AOASD55RGXG5O2IHCMK6YSTURCOSRANCNFSM5JYXZOQA ). Triage notifications on the go with GitHub Mobile for iOS ( https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 ) or Android ( https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub ).

bobf commented 2 years ago

@dorianmariefr Thanks a lot for the changes - I have just released 0.1.2: https://rubygems.org/gems/action_mailbox_amazon_ingress/versions/0.1.2

Joyeux noël !

dorianmariecom commented 2 years ago

Merci, joyeux noël à toi aussi 🎅

On Wed, Dec 15, 2021 at 15:17:34, bobf < @.*** > wrote:

@ dorianmariefr ( https://github.com/dorianmariefr ) Thanks a lot for the changes - I have just released 0.1.2 : https:/ / rubygems. org/ gems/ action_mailbox_amazon_ingress/ versions/ 0. 1. 2 ( https://rubygems.org/gems/action_mailbox_amazon_ingress/versions/0.1.2 )

Joyeux noël !

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub ( https://github.com/bobf/action_mailbox_amazon_ingress/pull/7#issuecomment-994833105 ) , or unsubscribe ( https://github.com/notifications/unsubscribe-auth/AOASD57VAGRU5KXVAAP3LFTURCPP5ANCNFSM5JYXZOQA ). Triage notifications on the go with GitHub Mobile for iOS ( https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 ) or Android ( https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub ).

skylarmacdonald commented 2 years ago

Hi,

Sorry to piggyback on this PR, but I don't think the '~> 6' constraint is doing what you want…

Bundler could not find compatible versions for gem "rails":
  In Gemfile:
    rails (~> 7.0.0)

action_mailbox_amazon_ingress (~> 0.1.2) was resolved to 0.1.2, which
depends on
      rails (~> 6)

From RubyGems guides:

It’s also important to know that if you specify a major version only, like this:
# gemspec
spec.add_runtime_dependency 'library', '~> 2'
It will only use the latest version from the 2.x series – so 2.3.0 – and not 3.0.0. This behaviour may surprise some people, but automatically allowing any major version past version 2 is more surprising behaviour.

bobf commented 2 years ago

@skylarmacdonald Interesting ! Thank you for the info. I don't know if I've ever used the pessimistic operator with a major version before so I made a wrong assumption about its behaviour.

It seems like the only options here are:

Set constraint to >= 6.1 (I remembered that 6.0 had support for AWS SES integration which was removed in 6.1 so the ~> 6 constraint is wrong for two reasons now).
Create a new release of the gem that only supports 7.x

I prefer the first option - do you have any thoughts ?

skylarmacdonald commented 2 years ago

The first option sounds good to me. I suspect there will be plenty of code out there in the wild that won't upgrade to Rails 7 yet, but will want to benefit from any future patches, so that would be my suggestion.

That's roughly what you were going for with the pessimistic operator, so makes sense to me.

dorianmariecom commented 2 years ago

oopsie, made a new PR

bobf commented 2 years ago

@skylarmacdonald @dorianmariefr Thanks, both - appreciated.

bobf / action_mailbox_amazon_ingress

allow upgrade to rails 7 #7