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.

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

Add the version constraint for actionpack #62

Merged
merged 2 commits into from
Jun 22, 2022
Merged

Add the version constraint for actionpack #62

merged 2 commits into from
Jun 22, 2022

Conversation

yykamei
Copy link
Contributor

@yykamei yykamei commented Jun 21, 2022

#38 introduced the check whether the route comes from Rails with route.app.engine?.

However, the method engine? was added in this commit, and this is available since Rails 5.2.

So, adding the version constraint might make sense.

Note Rails 5.2 reached EOL, which means patches against
severe security issues will not be available, so I added >= 6.0.0 instead of >= 5.2.0.

#38 introduced
the check whether the route comes from Rails with `route.app.engine?`.

However, the method `engine?` was added in this PR,
and this is available since Rails 5.2.

rails/rails@b50e88e

So, adding the version constraint might make sense.

Note Rails 5.2 reached EOL, which means patches against
severe security issues will not be available, so I added `>= 6.0.0`
instead of `>= 5.2.0`.
@k0kubun
Copy link
Collaborator

k0kubun commented Jun 21, 2022

I don't intend to maintain EOL Rails compatibility, but I don't find a strong motivation to drop Rails 5.2 compatibility either. If you only need >= 5.2.0, can you just specify >= 5.2.0 for now? I'll drop it when I need to.

@yykamei
Copy link
Contributor Author

yykamei commented Jun 22, 2022

Thank you for your feedback. I updated with de9bb86.

@k0kubun k0kubun merged commit ab93d1f into exoego:master Jun 22, 2022
@yykamei yykamei deleted the yykamei-patch-1 branch June 22, 2022 04:00
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