-
Notifications
You must be signed in to change notification settings - Fork 99
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
Let Administrate figure out the associations #44
Let Administrate figure out the associations #44
Conversation
Uh, CircleCI doesn't appear to be using Administrate |
Is the CI problem because of Appraisal? A thought here: we could point an Appraisal set to As for breaking changes: I think we're okay there, we'll just need to make a note in the CHANGELOG when we release. |
41226cb
to
161a584
Compare
Oh, I forgot about Appraisal. You are right, it works after removing the appraisals for old versions. So either this is merged without back compatibility, or I spend a bit more time to make it work with older versions. By the way, we already have some code to do back compatibility. If we are merging as is, we probably should also remove that: administrate-field-nested_has_many/app/views/fields/nested_has_many/_show.html.erb Lines 1 to 9 in bc1c881
Perhaps we can wait for the next version of Administrate and I'll do two PRs: one to remove compatibility with older versions, and another would be this PR. |
As mentioned I have opened PR #46, which will remove the compatibility with Administrate prior to v0.10 |
161a584
to
8cae5a5
Compare
Took this PR out of draft mode. I think this can be considered for merge now. |
Following thoughtbot/administrate#1633, this PR would bring this plugin to take advantage of Administrate's own association detection, doing away with the
:class_name
option.Note that this breaks compatibility with versions of Administrate prior to 0.15.