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

Add Rails 7 support #204

Merged
merged 12 commits into from
Jan 18, 2022
Merged

Add Rails 7 support #204

merged 12 commits into from
Jan 18, 2022

Conversation

daztmb
Copy link
Contributor

@daztmb daztmb commented Jan 10, 2022

Hi everyone!

According this conversation #198 we removed support for older versions of rails <5.2 and added support for 7 rails. We passed all the tests.

@ilyakatz could you please review this?

@ilyakatz
Copy link
Owner

Hi @daztmb, sorry, I'm a little confused. What is the relationship between #198 and this PR. Both look to be doing similar things or am I missing something?

@daztmb daztmb closed this Jan 11, 2022
@daztmb daztmb reopened this Jan 11, 2022
@daztmb
Copy link
Contributor Author

daztmb commented Jan 11, 2022

Hi @daztmb, sorry, I'm a little confused. What is the relationship between #198 and this PR. Both look to be doing similar things or am I missing something?

Hi @ilyakatz I only meant that we familiarizated with the conversation from that PR and took into account your wish to save support for rails 5.2

@ilyakatz
Copy link
Owner

sorry, still confused, the other PR also has support for 5.2, so are you saying two PR's do the same thing? You seem to be making better progress so if you think your PR does everything, I can ask #198 to close their PR in favor of yours

@daztmb
Copy link
Contributor Author

daztmb commented Jan 12, 2022

Yes, we have kept the rails support from 5.2 and up. It seems to us that we have already completed the upgrade of this gem. If you have any comments or suggestions for what we can improve, let us know and we will finalize it.

@@ -1,3 +1,3 @@
module DataMigrate
VERSION = "7.0.2".freeze
VERSION = "7.1.0".freeze
Copy link
Owner

Choose a reason for hiding this comment

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

since this is removing support for older versions of rails, this is a non-backward compatible change so it would need to be a major version bump. Also I think it should be a release candidate so that folks can test it out. So I think it should be 8.0.0.rc1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, thanks. Done

@ilyakatz
Copy link
Owner

Ok, sounds good. Just one small comment and after that we can deploy a release candidate version. After people try it out and we don't see any new reports, we can upgrade it to full major release versin

@daztmb daztmb requested a review from ilyakatz January 13, 2022 08:38
@ilyakatz
Copy link
Owner

sorry, one last thing, can you update https://github.com/ilyakatz/data-migrate/blob/master/Changelog.md and we'll be ready to go

@arkirchner
Copy link

@ilyakatz Now that the changelog is added can we release the RC candidate?

@ilyakatz ilyakatz merged commit c87e042 into ilyakatz:master Jan 18, 2022
@ilyakatz
Copy link
Owner

Merged, RC version is up on Rubygems! Once again, thanks for the PR!

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.

4 participants