-
Notifications
You must be signed in to change notification settings - Fork 21.9k
33515 invert remove foreign key support "to_table" #33530
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
33515 invert remove foreign key support "to_table" #33530
Conversation
Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @kamipo (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. This repository is being automatically checked for code quality issues using Code Climate. You can see results for this analysis in the PR status below. Newly introduced issues should be fixed before a Pull Request is considered ready to review. Please see the contribution instructions for more information. |
Pls, add info on :to_table option, it's missing, something like
|
CI
How to rerun? |
I restarted the failing job |
@schneems thanks |
Everything looks good. Can you squash to one commit. You can give credit to multiple authors using this technique https://contributors.rubyonrails.org/faq |
91634d9
to
54ef924
Compare
@schneems I just squash and force push. |
The real author name of #33519 is Nikolay Epifanov. https://patch-diff.githubusercontent.com/raw/rails/rails/pull/33519.patch |
ee4f4af
to
3692f93
Compare
@kamipo commit message is updated to use real author name |
Looking good. I forgot to mention that this is going to need a changelog since it’s a change in functionality. After that I think we are good to go. Thanks again for this. |
3692f93
to
a4a1914
Compare
CI was green. No code changes in this branch.
|
I restarted the tests |
Since that's merged can you rebase and re-push? |
a4a1914
to
1da0f7e
Compare
@schneems thanks, I rebase and force push. only one job fail, need restart this time |
1da0f7e
to
c0758d4
Compare
remove_foreign_key supports - remove_foreign_key :accounts, :branches - remove_foreign_key :accounts, to_table: :branches but the second one is not reversible. This branch is to fix and allow second one to be reversible. [Nikolay Epifanov, Rich Chen]
c0758d4
to
6b9948c
Compare
Thanks! |
- Move changelog entry of rails#33530 up in order to preserve the chronology since we always add new entries on the top of a changelog file. - Clarify the changelog entry - Clarify the docs of remove_foreign_key - Ensure reversible of `remove_foreign_key` with `:primary_key` and `:to_table` options.
@schneems thanks |
Summary
remove_foreign_key supports
but the second one is not reversible.
This branch is to fix and allow second one to be reversible.
Other Information
close #33515
continuing #33519, base on discussion between @NickTime and @sikachu