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

Lock Nokogiri version for Webrat monkey-patch #5745

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

Conversation

c960657
Copy link

@c960657 c960657 commented Dec 17, 2024

The monkey-patch #2469 in test/support/webrat/matchers.rb is not compatible with Nokogiri ≥ 1.17, so many tests are currently failing.

Nokogiri dropped support for Ruby 2.7 in 1.16 which is still supported by Devise, so locking Nokogiri to < 1.17 seems like the easiest fix.

@c960657
Copy link
Author

c960657 commented Dec 17, 2024

@jrichy1 I don't understand what you mean. Where exactly are you tagged?

@timdiggins
Copy link

I'm not a committer, but I don't think this PR is mergeable.

Locking devise to an older version of Nokogiri, a gem that gets a lot of security updates will lock out a lot of users for long ago compatibility support.

Better to drop ruby 2.7 (which is not compatible with latest Rails versions and went EOL in 2023: https://endoflife.date/ruby)

Also FYI #2469 is an issue not a PR, and has no attached code.

And as an aside, webrat (last update 10 years ago) needs to be dropped - not most likely with capybara - which is the logical upgrade path. But we'd need to see a lot of commiter enthusiasm before anyone would do a PR on that.

@c960657
Copy link
Author

c960657 commented Dec 19, 2024

Sorry, the link to #2469 was wrong. Should have been sparklemotion/nokogiri#2469.

Nokogiri is only used in tests, so there is no real security issue. But I agree that it is time to drop Ruby 2.7 support.

@timdiggins
Copy link

Nokogiri is only used in tests, so there is no real security issue.

Oops - yes missed that it was in Gemfile not gemspec... Sorry

But I agree that it is time to drop Ruby 2.7 support

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants