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 7 compatibility; fix CI pipeline #124

Merged
merged 4 commits into from
Sep 13, 2024
Merged

Conversation

padde
Copy link
Contributor

@padde padde commented Sep 11, 2024

While working on #121 I encountered many failures in the CI pipeline even without making any changes. It seems that the main reason for this is that the specs are now running on Rails 7. In particular:

  • ActiveSupport::Deprecation.warn raised an error
    • Calling warn on the singleton is itself deprecated1
    • The alternative is to create our own deprecator instance, which I have implemented
  • Rubocop complained about some outdated rules in .rubocop_todo.yml
    • I have removed the outdated entry
    • I also re-ran rubocop --auto-gen-config which updated some of the issue counts etc.
  • A handful of specs were failing for some combinations of the CI matrix
    • Apparently in some versions the HTTP header names will be lowercase
    • I changed the matchers to match with a case-insensitive Regex instead of a capitalized string
    • This should be fine as header names are case-insensitive2 anyway
  • CI Pipeline was fully green after these changes, already tested on my personal fork

Footnotes

  1. Deprecate ActiveSupport::Deprecation singleton usage rails/rails#47354

  2. "Each header field consists of a case-insensitive field name followed
    by a colon (":"), optional leading whitespace, the field value, and
    optional trailing whitespace." RFC 7230, Section 3.2

@dblock dblock merged commit bde8b9b into ruby-grape:master Sep 13, 2024
12 checks passed
@padde padde deleted the rails-7 branch September 19, 2024 21:45
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