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

Upgrade to 6.0 and 6.1 #135

Merged
merged 5 commits into from
Dec 19, 2023
Merged

Upgrade to 6.0 and 6.1 #135

merged 5 commits into from
Dec 19, 2023

Conversation

ashwinisukale
Copy link
Contributor

@ashwinisukale ashwinisukale commented Dec 15, 2023

Background:
As part of our Rails 6 upgrade process, I've encountered an issue related to the format: false constraint. In Rails 6, there have been changes in the way constraints are handled, specifically involving the use of Regexp.union(re).

Issue:
The particular error we're facing is a TypeError: No implicit conversion of false into String. This is occurring due to the introduction of Regexp.union(re) in Rails 6, which expects string values and does not accept boolean values.

Proposed Solution:
To address this issue, we're proposing the removal of the format: false constraint. In our investigation, we found that the Rails 6 update now relies on Regexp.union(re), which only accepts string values in constraints.

Testing:
We have already added test cases to validate the behavior after removing the format: false constraint. These tests are aimed at ensuring the smooth functioning of our application under the Rails 6 environment.

Links

Rails 6.0 -
https://github.com/rails/rails/blob/6-0-stable/actionpack/lib/action_dispatch/journey/path/pattern.rb#L93

Rails 5.2 -
https://github.com/rails/rails/blob/5-2-stable/actionpack/lib/action_dispatch/journey/path/pattern.rb#L93

@ashwinisukale ashwinisukale self-assigned this Dec 15, 2023
Copy link
Contributor

@richardhallett richardhallett left a comment

Choose a reason for hiding this comment

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

That all sounds to make sense to me.
After deploy a few manual checks of the content negotiation service would be good too, thanks.

@ashwinisukale ashwinisukale merged commit 7f25529 into master Dec 19, 2023
3 checks passed
@ashwinisukale ashwinisukale deleted the upgrade_to_6 branch December 19, 2023 15:16
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