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

drop support for deprecated rails versions #431

Merged
merged 1 commit into from
Apr 4, 2018

Conversation

grosser
Copy link
Contributor

@grosser grosser commented Apr 1, 2018

No description provided.

@fatkodima
Copy link
Contributor

This also can be removed

revision.send :instance_variable_set, '@attributes', self.attributes if rails_below?('4.2.0')

@grosser
Copy link
Contributor Author

grosser commented Apr 1, 2018

done, good to go ?

@fatkodima
Copy link
Contributor

fatkodima commented Apr 1, 2018

Looks like readme should be changed here

Audited currently (4.x) works with Rails 5.1, 5.0 and 4.2. It may work with 4.1 and 4.0, but this is not guaranteed.

And added missing changelog entry.

Also this helper is not needed anymore

Rails::VERSION::MAJOR == 4 ? 'ActiveRecord::Migration' : "ActiveRecord::Migration[#{ActiveRecord::Migration.current_version}]"

@grosser
Copy link
Contributor Author

grosser commented Apr 1, 2018

helper is still needed since I'm not removing rails 4.2

@grosser
Copy link
Contributor Author

grosser commented Apr 1, 2018

updated and green

@grosser
Copy link
Contributor Author

grosser commented Apr 3, 2018

review plz

@fatkodima
Copy link
Contributor

@tbrisker Did you proposed to delete all 4.x (so 4.2 as well) rails versions, or just 4.0 and 4.1?

@tbrisker
Copy link
Collaborator

tbrisker commented Apr 4, 2018

I suggested dropping all in #430, but perhaps it would be better to split it in two and keep just 4.2 in the stable 4.x branch, considering that 4.{0,1} have not been officially supported for some time.

Copy link
Collaborator

@tbrisker tbrisker left a comment

Choose a reason for hiding this comment

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

Looks good to me, I'll wait a few days before merging in case some folks have further input or disagreement in #430 regarding this.

@grosser
Copy link
Contributor Author

grosser commented Apr 4, 2018 via email

@tbrisker
Copy link
Collaborator

tbrisker commented Apr 4, 2018

Considering we haven't been supporting them officially for at least 2 years, I guess it's safe to drop. Thanks @grosser !

@tbrisker tbrisker merged commit 48f968d into collectiveidea:master Apr 4, 2018
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