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

Rails 8.0 support #182

Merged
merged 2 commits into from
Dec 31, 2024
Merged

Rails 8.0 support #182

merged 2 commits into from
Dec 31, 2024

Conversation

jprosevear
Copy link
Contributor

Issue #, if available:

Summary

Other Information

@jprosevear jprosevear marked this pull request as draft November 17, 2024 21:27
@coveralls
Copy link

Coverage Status

coverage: 99.737% (-0.3%) from 100.0%
when pulling 2e1d027 on codalio:chore/rails-8-0
into ea1e1b1 on simukappu:master.

2 similar comments
@coveralls
Copy link

Coverage Status

coverage: 99.737% (-0.3%) from 100.0%
when pulling 2e1d027 on codalio:chore/rails-8-0
into ea1e1b1 on simukappu:master.

@coveralls
Copy link

Coverage Status

coverage: 99.737% (-0.3%) from 100.0%
when pulling 2e1d027 on codalio:chore/rails-8-0
into ea1e1b1 on simukappu:master.

@coveralls
Copy link

coveralls commented Nov 17, 2024

Coverage Status

coverage: 100.0%. remained the same
when pulling cccc4d7 on codalio:chore/rails-8-0
into ea1e1b1 on simukappu:master.

@@ -153,7 +153,7 @@ def notify_to(*resources)
if options[:with_devise].present? && options[:devise_default_routes].present?
create_notification_routes options, resources_options
else
self.resources target, only: :none do
self.resources target, only: [] do
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is because of rails/rails#51464 - all tests now pass, but I'm unsure of your intention @simukappu with this code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hi @simukappu any feedback?

Copy link
Owner

Choose a reason for hiding this comment

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

I'm sorry for my very late response. I'm not sure with my original intent, but this modification seems fine. I'll merge your PR and update this gem. Thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@simukappu no problem, this gem is much appreciated.

@simukappu simukappu mentioned this pull request Dec 31, 2024
@simukappu simukappu marked this pull request as ready for review December 31, 2024 06:28
@simukappu simukappu merged commit e7631d8 into simukappu:master Dec 31, 2024
26 checks passed
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.

3 participants