Skip to content

Remove support for old Rails versions #617

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

Merged
merged 5 commits into from
Feb 9, 2021
Merged

Remove support for old Rails versions #617

merged 5 commits into from
Feb 9, 2021

Conversation

deivid-rodriguez
Copy link
Contributor

I noticed several CI failures on #616. Since all of them happen on Rails versions no longer maintain, I think that's a good sign that it's time to drop support.

@deivid-rodriguez deivid-rodriguez changed the title Remove support for Rails Remove support for old Rails versions Jun 10, 2020
@deivid-rodriguez
Copy link
Contributor Author

I went crazy and also dropped support for ruby 2.4, which is also unsupported upstream. CI is green now.

Feel free to cherry-pick any of these changes if they are of interest :)

@elia
Copy link
Contributor

elia commented Oct 9, 2020

@guilleiguaran or anyone else from the core team, I think this one should be reviewed to put the CI back on track
@deivid-rodriguez can you add a note on how the commit you reverted was breaking the build? never mind, it's available in full-detail at #621 👍

@deivid-rodriguez
Copy link
Contributor Author

Coming here from #629 that just got merged! 🎉

Now, in order to get CI green, I see a couple of ways:

  • Drop support for whatever it's failing, since it's unsupported upstream anyways. This is what this PR does and I see it as "the easy way" in terms of implementation but "the hard way" in terms of decision making, since a policy for dropping support for old Rails and Rubies need to be decided.

  • Fix things. This is what, at least partially, Fix acceptance test for under rails 5.1 #630 does. This is harder but less debatable since it doesn't involve dropping support for anything.

Thanks @Sixeight for your work by the way 😃.

Copy link
Contributor

@elia elia left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this, I hope the project will be soon back to green ✅
Just a nit over not using RC versions, which I've found in the recent past to be quite deceptive.

- 2.5.5
- 2.6.3
- ruby-head
env:
- RAILS_VERSION="~> 4.2.0"
- RAILS_VERSION="~> 5.0.0"
- RAILS_VERSION="~> 5.1.0"
- RAILS_VERSION="~> 5.2.0"
- RAILS_VERSION="~> 6.0.0.rc1"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this can be bumped to ~> 6.0.0 now

@deivid-rodriguez
Copy link
Contributor Author

Thanks for the review @elia! I observed the same thing, and also that ruby 2.7 is not currently tested.

I was expecting to get some feedback here and then maybe make those updates separately on a different PR, but I can also add that change here if maintainers prefer. I was just afraid that something might fail and then I have to fix it, then the PR starts growing and I keep going down the rabbit hole. I think dropping support is a good scope for this PR.

@deivid-rodriguez
Copy link
Contributor Author

Hi again maintainers :)!

I was thinking that, given that Rails itself has a quite aggressive policy regarding supported rubies (which I agree with), I think dropping support for ruby 2.4 seems fine? Since Rails 6, only ruby 2.5 and above are supported by Rails itself.

Regarding dropping support for old Rails versions, I believe it's also fine since Rails no longer supports any of those.

So my thinking is that it's fine to do this and ship it with the next minor release, but if maintainers prefer major version for this, it's also fine.

I'd very much like the CI to be back to green to avoid further accidental regressions like #621.

Thoughts?

@deivid-rodriguez
Copy link
Contributor Author

Hello again!

It's been a couple of months already, so I'd like to offer any help needed to get things moving forward.

@kaspth kaspth merged commit 24d9e93 into rails:master Feb 9, 2021
@kaspth
Copy link
Contributor

kaspth commented Feb 9, 2021

Awesome, thank you!

@deivid-rodriguez deivid-rodriguez deleted the remove_old_rails branch February 9, 2021 20:51
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