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

Upgrade for rails7 - WIP #198

Closed
wants to merge 4 commits into from

Conversation

aka47
Copy link

@aka47 aka47 commented Dec 14, 2021

added rails7 to the spec setup. But rails 7 changes a lot of internals around migrations - this needs a lot more work so that specs will work in rails 7 and older versions as well..

it breaks all other rails versions
@ilyakatz
Copy link
Owner

thanks for starting this! to be honest, i've been out of rails land for almost two years so not sure how much I can help. but can try to dig in my memory if help is needed

@lewhit are you still involved in rails projects? could I ask for your help here?

@aka47
Copy link
Author

aka47 commented Dec 15, 2021

thanks for starting this! to be honest, i've been out of rails land for almost two years so not sure how much I can help. but can try to dig in my memory if help is needed

@lewhit are you still involved in rails projects? could I ask for your help here?

Thanks for jumping in.
So this project needs a new maintainer? Is that what you are saying?

I believe for rails 7 the codebase will become simpler, but with the support for the older rails version - it will not get simpler all together. But it might make sense to support only 6+7 if this helps find a new maintainer..

it needs a lot more work, cleanup, spec fixes. Rails 4 and 5 support should get removed. It is just too complex.
@ilyakatz
Copy link
Owner

yeah, would be nice to get more people involved into maintenance of this gem. so if you go through this this PR and are interested in helping out later, i'd appreciate it. If not, that's ok. Anyways, if needed, I can help get this release out.

As for support, I see that rails 5.2 is still active so (https://endoflife.date/rails) so I think it makes sense to keep the support . Once that goes away, the gem can be updated

@bryanwoods
Copy link

@aka47 let me know if my team can help support getting this PR over the line. It looks like this is our primary blocker to a Rails 7 upgrade. Not sure from looking at the Travis build if you have a clear path forward or if there's still discovery to be done.

@aka47
Copy link
Author

aka47 commented Dec 23, 2021

@aka47 let me know if my team can help support getting this PR over the line. It looks like this is our primary blocker to a Rails 7 upgrade. Not sure from looking at the Travis build if you have a clear path forward or if there's still discovery to be done.

Help is very welcome, the whole gem and codebase is very knew to me. My first start was to fix what is broken. But I am not sure this is the best approach.
The codebase is very complex, with every rails release there is a new monkey patch that needs to be created. Plus the standard patch is the one for rails5. That makes it all very confusing.

One approach, and I would say the best, would be to create a branch for v2 ,which is supporting Rails-7 only. Remove all the old patches and just build it for one this rails version. That should simplify everything and make it good maintainable for future rails releases.
Support for rails 5 - 6 would be still available within the data-migration branch/releases v1.xx . Rails 5.2 will also be out of support in 5 month, so this is mainly for rails 6 and old rails 4/5 installations.

A lot of gems who need to work very close with rails internals do it that way. If we can agree on the way forward, that would be step one. Step 2 would be cleanup and rails 7 integration.

What are your thoughts on this? @ilyakatz @bryanwoods @lewhit

@ilyakatz
Copy link
Owner

hey @aka47, since this gem has very specific functionality, there are not many new features that were traditionally added. so what that means in practice, if we stop supporting an older version of rails in a major release, there are unlikely to be new features added that people will miss. And since 5.2 will stop support soon, I'd be ok with dropping support of rails 5.

it would be nice to have support for rails 6 together with 7, but if you think it's too complex, i'm fine with cutting it off. So v7 of data-migrate will support Rails 6 and down, and v8 will support Rails 7.

I thought I removed Rails 4 support before, if not, feel free to get rid of those.

Is this helpful?

@aka47
Copy link
Author

aka47 commented Dec 23, 2021

Is this helpful?

yes -- having a shared plan forward is a great first step. Once I am back from xmas holidays - I will start with the cleanup and a first structure to integrate data-migrate with the new public API from Rails for migrations. Since the API is public now - it might not change that much in future releases of rails.

@bryanwoods - if you want to jump in as well with your team, that would be great. Cleaning up the old monkey patches and build an integration with the Rails 7 API are the 2 main tasks to be done.

@daztmb daztmb mentioned this pull request Jan 10, 2022
@ilyakatz ilyakatz closed this Jan 29, 2022
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