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

Respect configured primary key type in migration #836

Merged
merged 4 commits into from
Aug 9, 2023

Conversation

MSchmidt
Copy link
Contributor

@MSchmidt MSchmidt commented Aug 9, 2023

Instead of changing the migration manually this will respect the setting from the rails project. Implementation is the same as ActiveStorage uses.

@excid3
Copy link
Collaborator

excid3 commented Aug 9, 2023

This is great @MSchmidt! Didn't realize ActiveStorage had this and had wanted a solution for a while like this.

@MSchmidt
Copy link
Contributor Author

MSchmidt commented Aug 9, 2023

Oh you are absolutely right. I adjusted this in my project and then messed the PR up. Will fix

@MSchmidt
Copy link
Contributor Author

MSchmidt commented Aug 9, 2023

PR now adjusted for standardrb and remove a note from docs that does not apply anymore.

@excid3 excid3 merged commit ea73cf8 into pay-rails:master Aug 9, 2023
33 checks passed
@excid3
Copy link
Collaborator

excid3 commented Aug 9, 2023

Perfect. Thanks again for this. Super duper handy and nice to see that Rails uses this approach internally!

@MSchmidt
Copy link
Contributor Author

MSchmidt commented Aug 9, 2023

Thanks for merging so quickly. Wondering if this should be part of ActiveRecord::Migration then. Not very DRY if every project needs the same few lines of code.

@excid3
Copy link
Collaborator

excid3 commented Aug 9, 2023

Sure seems like it could be extracted as a couple methods for easy access because any Rails engine with migrations should use it. Maybe you could get it in before Rails 7.1?

Tag me on the PR if you make one to Rails!

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

2 participants