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

Remove conditional code that targets Rails 5.1 #3505

Merged
merged 5 commits into from
Feb 19, 2020

Conversation

kennyadsl
Copy link
Member

Description

🧹🧹🧹🧹

We still have some conditional code that targets Rails 5.1, which has been deprecated and we do not support anymore.

This PR removes that code in order to keep the codebase as clean as possible.

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 added the type:enhancement Proposed or newly added feature label Feb 4, 2020
@kennyadsl kennyadsl self-assigned this Feb 4, 2020
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

Thanks

unless RAILS_52_OR_ABOVE
protect_from_forgery with: :exception
end
protect_from_forgery with: :exception
Copy link
Member

Choose a reason for hiding this comment

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

This code was executed only when RAILS_52_OR_ABOVE was false, so I think the line should be removed

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right!

Adding some longer config would result in changing
all of them and breaking git history everytime.
It was under a conditional block but now it makes no sense
to have it separate from the other configurations.
@kennyadsl kennyadsl force-pushed the kennyadsl/remove-rails-5-1-code branch from 5c01338 to badd349 Compare February 19, 2020 13:31
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.

@kennyadsl thank you 👍

@kennyadsl kennyadsl merged commit 728aa69 into solidusio:master Feb 19, 2020
@kennyadsl kennyadsl deleted the kennyadsl/remove-rails-5-1-code branch February 19, 2020 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:enhancement Proposed or newly added feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants