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.

Sign up for GitHub

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

Jump to bottom

Update canonical-rails syntax for latest version #3865

Merged
merged 1 commit into from
Jan 7, 2021
Merged

Update canonical-rails syntax for latest version #3865

merged 1 commit into from
Jan 7, 2021

Conversation

brchristian
Copy link
Contributor

@brchristian brchristian commented Dec 13, 2020 •
edited
Loading

Description
canonical-rails version 0.2.10 changed the method whitelisted_parameters to allowed_parameters. (See https://github.com/jumph4x/canonical-rails/pull/58/files.)

Any user running bundle update and going from version <= 0.2.9 to 0.2.10 will fail to compile their asset pipeline, resulting in the error NoMethodError: undefined method 'whitelisted_parameters=' for CanonicalRails:Module.

This updates to the current syntax, and also sets the dependency at 0.2.10 to prevent any incompatibility issues that might arise from using the new allowed_parameters with old versions that still expect whitelisted_parameters.

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have updated Guides and README accordingly to this change (if needed)
  • I have added tests to cover this change (if needed)
  • I have attached screenshots to this PR for visual changes (if needed)

@kennyadsl kennyadsl mentioned this pull request Dec 14, 2020
Merged
@kennyadsl
Copy link
Member

kennyadsl commented Dec 14, 2020 •
edited
Loading

Thanks, I created jumph4x/canonical-rails#65 to avoid backporting this to old Solidus versions. I think we should wait for their next release if this is accepted.

cc @jumph4x

@kennyadsl kennyadsl mentioned this pull request Dec 14, 2020
Merged
5 tasks
@brchristian
Copy link
Contributor Author

@kennyadsl I like your solution even better!

@kennyadsl
Copy link
Member

@brchristian they are both needed otherwise we'll have a deprecation warning here! Thanks!

@jumph4x
Copy link
Contributor

jumph4x commented Dec 14, 2020

I see!

@jumph4x
Copy link
Contributor

jumph4x commented Dec 14, 2020 •
edited
Loading

Thank you for the contribution. I'll cut a patch version release to make sure existing Gemfiles will continue working. Sorry for breaking it!

Sinetheta added a commit to Sinetheta/solidus_graphql_api that referenced this pull request Dec 14, 2020
@Sinetheta
Restrict canonical-rails until patches can roll out
8077e1f 
canonical-rails introduced a breaking change in 0.2.10 which is
causing some Solidus problems on master.

solidusio/solidus#3865
Sinetheta added a commit to Sinetheta/solidus_graphql_api that referenced this pull request Dec 14, 2020
@Sinetheta
Restrict canonical-rails until patches can roll out
c07bb76 
canonical-rails introduced a breaking change in 0.2.10 which is
causing some Solidus problems on master.

solidusio/solidus#3865
Sinetheta added a commit to Sinetheta/solidus_graphql_api that referenced this pull request Dec 14, 2020
@Sinetheta
Restrict canonical-rails until patches can roll out
992995a 
canonical-rails introduced a breaking change in 0.2.10 which is
causing some Solidus problems on master.

solidusio/solidus#3865
Sinetheta added a commit to Sinetheta/solidus_graphql_api that referenced this pull request Dec 14, 2020
@Sinetheta
Restrict canonical-rails until patches can roll out
2fbba30 
canonical-rails introduced a breaking change in 0.2.10 which is
causing some Solidus problems on master.

solidusio/solidus#3865
@Sinetheta Sinetheta mentioned this pull request Dec 14, 2020
Merged
@jumph4x
Copy link
Contributor

jumph4x commented Dec 15, 2020

https://rubygems.org/gems/canonical-rails/versions/0.2.11

@peterberkenbosch
Copy link
Contributor

nice. My bad for not adding deprecations. Thank @kennyadsl for fixing that!

@peterberkenbosch
Copy link
Contributor

This is good to go right? Would love to start new project on Rails 6.1 / Solidus master ;) So let me know if there is anything I can do to move this forward.

kennyadsl
kennyadsl reviewed Dec 21, 2020
View reviewed changes
frontend/solidus_frontend.gemspec
@@ -24,7 +24,7 @@ Gem::Specification.new do |s|
s.add_dependency 'solidus_api', s.version
s.add_dependency 'solidus_core', s.version

s.add_dependency 'canonical-rails', '~> 0.2.0'
s.add_dependency 'canonical-rails', '~> 0.2.10'
Copy link
Member

Choose a reason for hiding this comment

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

we can probably revert this gemspec change now?

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 PR moves from the deprecated whitelisted_params to the new allowed_params, which works in 0.2.10 and 0.2.11, so I think that we are going to want something to lock the version at 0.2.10 or 0.2.11 in order to ensure compatibility. The allowed_params method is not compatible with 0.2.9 and lower.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, you are totally right.

spaghetticode
spaghetticode approved these changes Dec 22, 2020
View reviewed changes
Copy link
Member

@spaghetticode spaghetticode left a comment

Choose a reason for hiding this comment

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

@brchristian thank you, and thanks also to @kennyadsl for his PR on canonical-rails!

@peterberkenbosch
Copy link
Contributor

Can we merge this? Or waiting for the backport?

@kennyadsl kennyadsl merged commit 14f43d8 into solidusio:master Jan 7, 2021
@brchristian brchristian deleted the canonical branch January 7, 2021 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gmacdougall gmacdougall gmacdougall approved these changes

@kennyadsl kennyadsl kennyadsl approved these changes

@spaghetticode spaghetticode spaghetticode approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@brchristian @kennyadsl @jumph4x @peterberkenbosch @spaghetticode @gmacdougall