Remove legacy mysql adapter #22715

Merged
merged 1 commit into from Dec 21, 2015

Conversation

Projects
None yet
6 participants
@kamipo
Member

kamipo commented Dec 20, 2015

Follow up to #22642.

@rails-bot

This comment has been minimized.

Show comment
Hide comment
@rails-bot

rails-bot Dec 20, 2015

r? @rafaelfranca

(@rails-bot has picked a reviewer for you, use r? to override)

r? @rafaelfranca

(@rails-bot has picked a reviewer for you, use r? to override)

@sgrif

This comment has been minimized.

Show comment
Hide comment
@sgrif

sgrif Dec 21, 2015

Member

I'd like to re-raise this issue. I'd like to propose that we revert #22642 and hold of on merging this until master is tracking 5-1-stable. There's very little cost to us maintaining this for a few more weeks, and we can save our users that are still using the legacy adapter a full version before they have to deal with upgrade pain. It would also give us more time to deal with getting it extracted to a gem that someone else can maintain.

Member

sgrif commented Dec 21, 2015

I'd like to re-raise this issue. I'd like to propose that we revert #22642 and hold of on merging this until master is tracking 5-1-stable. There's very little cost to us maintaining this for a few more weeks, and we can save our users that are still using the legacy adapter a full version before they have to deal with upgrade pain. It would also give us more time to deal with getting it extracted to a gem that someone else can maintain.

@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Dec 21, 2015

Member

There's very little cost to us maintaining this for a few more week

It is not a few week. It is until the Rails 5.2 or whatever the N+2 release will be called. Don't forget that we still support the stable release and sometimes two stable releases, what means that we may support it for more that 2 years.

Member

rafaelfranca commented Dec 21, 2015

There's very little cost to us maintaining this for a few more week

It is not a few week. It is until the Rails 5.2 or whatever the N+2 release will be called. Don't forget that we still support the stable release and sometimes two stable releases, what means that we may support it for more that 2 years.

@sgrif

This comment has been minimized.

Show comment
Hide comment
@sgrif

sgrif Dec 21, 2015

Member

I'm fine with being responsible for bugfixes for however long 5-0-stable is supported.

Member

sgrif commented Dec 21, 2015

I'm fine with being responsible for bugfixes for however long 5-0-stable is supported.

@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Dec 21, 2015

Member

@sgrif I'm fine with that. Of course I don't mind to support too, but we would have removed it in Rails 4.2 and we didn't finished. I don't see why keep it in Rails 5. People can upgrade to mysql2 adapter before upgrading to Rails 5. Or we can finish the extraction to a gem before the Rails 5 final. I prefer to extract it now and I think it is possible if someone focus on that.

Member

rafaelfranca commented Dec 21, 2015

@sgrif I'm fine with that. Of course I don't mind to support too, but we would have removed it in Rails 4.2 and we didn't finished. I don't see why keep it in Rails 5. People can upgrade to mysql2 adapter before upgrading to Rails 5. Or we can finish the extraction to a gem before the Rails 5 final. I prefer to extract it now and I think it is possible if someone focus on that.

@sgrif

This comment has been minimized.

Show comment
Hide comment
@sgrif

sgrif Dec 21, 2015

Member

@rafaelfranca Don't get me wrong. The adapter has been painful to keep around and I'm happy to see it go, and I'm definitely in favor of extracting it. But it doesn't seem like there's much work on that front, and I'd like to make sure that anyone still using it has a straightforward migration path for Rails 5 (assuming that just switching to mysql2 isn't necessarily an option).

I guess for now let's go ahead and continue as we have, and if the issue hasn't been resolved by the time we get into RCs we can revisit.

Member

sgrif commented Dec 21, 2015

@rafaelfranca Don't get me wrong. The adapter has been painful to keep around and I'm happy to see it go, and I'm definitely in favor of extracting it. But it doesn't seem like there's much work on that front, and I'd like to make sure that anyone still using it has a straightforward migration path for Rails 5 (assuming that just switching to mysql2 isn't necessarily an option).

I guess for now let's go ahead and continue as we have, and if the issue hasn't been resolved by the time we get into RCs we can revisit.

sgrif added a commit that referenced this pull request Dec 21, 2015

@sgrif sgrif merged commit 5609958 into rails:master Dec 21, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Dec 21, 2015

Member

@sgrif agree. Let make sure we have it extracted before the RC. @seuros mind to work on that?

Member

rafaelfranca commented Dec 21, 2015

@sgrif agree. Let make sure we have it extracted before the RC. @seuros mind to work on that?

@kamipo kamipo deleted the kamipo:remove_mysql_adapter branch Dec 21, 2015

@matthewd

This comment has been minimized.

Show comment
Hide comment
Member

matthewd commented Jan 8, 2016

@bronson 👍

bronson added a commit to bronson/rails that referenced this pull request Jan 8, 2016

@bronson

This comment has been minimized.

Show comment
Hide comment
@bronson

bronson Jan 8, 2016

Contributor

OK, it's in #22983

Doing what I can to discourage this PR from being reverted. :)

Contributor

bronson commented Jan 8, 2016

OK, it's in #22983

Doing what I can to discourage this PR from being reverted. :)

bronson added a commit to bronson/rails that referenced this pull request Jan 9, 2016

bronson added a commit to bronson/rails that referenced this pull request Jan 9, 2016

bronson added a commit to bronson/rails that referenced this pull request Jan 9, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment