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 code related to Rails < 5.0 & Ruby < 2.3.0 #2226

Merged
merged 1 commit into from Dec 20, 2019

Conversation

pirj
Copy link
Member

@pirj pirj commented Dec 10, 2019

Another cleanup attempt (former: #2225).

Note: in the Changelog, we state that we drop support for Ruby versions below 2.3, even though Rails 5.2 supports Ruby 2.2.2. It's a matter of getting just a couple lines back to keep 2.2.2 support.

I didn't dare to refactor FixtureFileUploadSupport.

@pirj pirj self-assigned this Dec 10, 2019
@pirj pirj force-pushed the cleanup-unsupported-ruby-and-rails-4-checks branch 2 times, most recently from 2052a6d to 8062d3f Compare December 10, 2019 23:52
@pirj

This comment has been minimized.

@pirj pirj marked this pull request as ready for review December 11, 2019 17:03
@pirj pirj force-pushed the cleanup-unsupported-ruby-and-rails-4-checks branch from bde195b to 6020cc2 Compare December 11, 2019 17:15
Gemfile Outdated Show resolved Hide resolved
@pirj
Copy link
Member Author

pirj commented Dec 11, 2019

@JonRowe It's a big one, but mostly removals of old Rubies/Rails 3/4 support. Appreciate if you glance over.

@pirj pirj force-pushed the cleanup-unsupported-ruby-and-rails-4-checks branch from f679002 to 01567bf Compare December 11, 2019 20:32
@benoittgt
Copy link
Member

For Rails 4.2 did you read this comment #2071 (comment)
I am wondering how much we should clean code related to Rails 4.2

Thanks for the work.

@pirj
Copy link
Member Author

pirj commented Dec 13, 2019

@benoittgt Thanks for pointing up, I completely missed the second part of this comment.
I'll check again if there's something pre-4.2 that can be dropped.

@pirj pirj closed this Dec 13, 2019
@pirj pirj deleted the cleanup-unsupported-ruby-and-rails-4-checks branch December 13, 2019 16:46
@JonRowe
Copy link
Member

JonRowe commented Dec 14, 2019

Pretty much all of this was good to go @pirj I'd reopen it as is mostly.

At 4.0 release time, 4.2 will be removed from the build matrix, but not the gem spec

This means we can remove all of our spec / feature tests for Rails 4.2

4.2 will not be officially supported in RSpec Rails 4, and bugs against that version will not be fixed

This means that we should leave the checks for 4.2 methods to check they are supported before calling in lib, and the same for files. However anything that is old Ruby is ok to be dropped. We're hard supporting 5.0 and soft supporting 4.2 but not old Ruby, thats gone.

A warning will be issued at require time, if a user is using Rails 4.2, indicating to them that while the version is unlikely to be supported, bug fixes will not be issued against 4.2

This needs to be added somewhere.

@benoittgt
Copy link
Member

benoittgt commented Dec 14, 2019

Sorry @pirj I was maybe not clear enough. But I don't this PR should be closed. :)

@benoittgt benoittgt restored the cleanup-unsupported-ruby-and-rails-4-checks branch December 14, 2019 13:31
@pirj pirj reopened this Dec 16, 2019
@JonRowe
Copy link
Member

JonRowe commented Dec 17, 2019

@pirj let me know when this is ready to review again :)

@pirj pirj force-pushed the cleanup-unsupported-ruby-and-rails-4-checks branch 3 times, most recently from a552b5e to 3218bd4 Compare December 17, 2019 19:47
@pirj
Copy link
Member Author

pirj commented Dec 17, 2019

@JonRowe @benoittgt Please take another look.

I broke four examples on 4.2, mailer_generator_spec.rb and configuration_spec.rb, by removing < 5.0 checks in those specs. The code should work fine. If you comment them out

RAILS_VERSION=4.2.11 rake

expectedly get stuck at the point where @rails_post_5 tags were.

Copy link
Member

@JonRowe JonRowe left a comment

Choose a reason for hiding this comment

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

It looks good but I want to hold off final review until we've got green CI

example_app_generator/generate_app.rb Show resolved Hide resolved
@pirj pirj force-pushed the cleanup-unsupported-ruby-and-rails-4-checks branch from 3218bd4 to 0df1820 Compare December 20, 2019 18:42
@pirj
Copy link
Member Author

pirj commented Dec 20, 2019

@JonRowe @benoittgt Green!

@JonRowe
Copy link
Member

JonRowe commented Dec 20, 2019

One change really, only due to our process, currently the dev build should apply cleanly to rspec-rails and I'd like to keep it that way for centrally managed files

@pirj pirj force-pushed the cleanup-unsupported-ruby-and-rails-4-checks branch from 0df1820 to fc959be Compare December 20, 2019 21:22
@pirj pirj force-pushed the cleanup-unsupported-ruby-and-rails-4-checks branch from fc959be to d52a135 Compare December 20, 2019 21:43
@pirj
Copy link
Member Author

pirj commented Dec 20, 2019

All green. Reverted changes to predicate functions. Squashed commits.

@JonRowe JonRowe merged commit a769a4d into master Dec 20, 2019
@JonRowe JonRowe deleted the cleanup-unsupported-ruby-and-rails-4-checks branch December 20, 2019 23:14
JonRowe added a commit that referenced this pull request Jan 12, 2020
…s-4-checks

Drop code related to Rails < 5.0 & Ruby < 2.3.0
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

3 participants