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

Fix ActiveRecord version check in Rails 4.0 #988

Merged

Conversation

magnusvk
Copy link
Contributor

@magnusvk magnusvk commented Sep 5, 2017

Rails 4.0 does not have the ActiveRecord.gem_version method, so this
check will fail there. However, we know that if gem_version is not
available, we are not dealing with Rails >= 5.0, so this will work in
both cases.

See #956 for an equivalent case elsewhere in the codebase.

Rails 4.0 does not have the `ActiveRecord.gem_version` method, so this
check will fail there. However, we know that if `gem_version` is not
available, we are not dealing with Rails >= 5.0, so this will work in
both cases.

See paper-trail-gem#956 for an equivalent case elsewhere in the codebase.
magnusvk added a commit to magnusvk/counter_culture that referenced this pull request Sep 5, 2017
As they currently fail. We should be able to remove this if
paper-trail-gem/paper_trail#988 gets merged.
Copy link
Member

@jaredbeck jaredbeck left a comment

Choose a reason for hiding this comment

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

Rails 4.0 does not have the ActiveRecord.gem_version method, so this
check will fail there. However, we know that if gem_version is not
available, we are not dealing with Rails >= 5.0, so this will work in
both cases.

Thanks Magnus. Looks good to me. Please add:

  • changelog entry under Unreleased -> Fixed
  • comment above conditional explaining that conditional can be deleted when we drop support (soon) for rails < 4.2

@magnusvk
Copy link
Contributor Author

magnusvk commented Sep 7, 2017

@jaredbeck added as requested.

CHANGELOG.md Outdated
@@ -31,6 +31,8 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/).

- [#985](https://github.com/airblade/paper_trail/pull/985) - Fix RecordInvalid
error on nil item association when belongs_to_required_by_default is enabled.
- [#988](https://github.com/airblade/paper_trail/pull/988) - Fix ActiveRecord
version check in `VersionConcern` for Rails 4.0
Copy link
Member

Choose a reason for hiding this comment

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

Please put your changelog entry under Unreleased -> Fixed. Here you have it under 7.1.2 -> Fixed. Thanks.

@magnusvk
Copy link
Contributor Author

magnusvk commented Sep 7, 2017

Oops. @jaredbeck fixed.

@jaredbeck jaredbeck merged commit c783109 into paper-trail-gem:master Sep 7, 2017
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.

None yet

2 participants