-
Notifications
You must be signed in to change notification settings - Fork 15
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
NEW Add migration task from gorriecoe/silverstripe-link #253
NEW Add migration task from gorriecoe/silverstripe-link #253
Conversation
b992618
to
3dff66f
Compare
3dff66f
to
6346428
Compare
ed5fd39
to
29e52b7
Compare
6759999
to
6466b5b
Compare
07293fe
to
9d51b6a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally, seems really good, only some minor things
is_enabled: true | ||
``` | ||
|
||
- Declare any columns that you added to the gorriecoe link model which need to be migrated to the new base link table, for example if you added a custom sort column for your `has_many` relations: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would make sense to move the yml config up much higher and be above where the diff
examples are, because when you run through this you've removed all of the old config by this point, yet with the yml you're asked to define it, though you have nothing to refer to
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to keep all of the migration task configuration in one chunk - are you suggesting moving all of the migration task configuration up above the rest of the setup?
I'm alright with doing that (and I'll update the v2/v3 to v4 migration docs while I'm at it) if that's what you want, but just want to clarify before doing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah move the migration task config above the bit where it tells you to delete old stuff. Because you need to refer to that old stuff when adding your migration task config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
I've also moved the "Take a backup of your database" into its own callout block to highlight that a bit better - it's easier to miss when its a step buried in a section like that, especially for experienced devs who may want to just skimread the docs and then figure it out themselves.
9d51b6a
to
2d0e3c8
Compare
2d0e3c8
to
7117247
Compare
Description
Adds a migration task and guide for migrating from
gorriecoe/silverstripe-link
andgorriecoe/silverstripe-linkfield
Requires silverstripe/silverstripe-framework#11182 for CI to go green.
Manual testing steps
gorriecoe/silverstripe-link
has_one
has_many
(with custom sort column)many_many
(withmany_many_extraFields
)many_many
through (with extra$db
fields)many_many
through, with polymorphichas_one
on the "from" end of the join modelcomposer require silverstripe/linkfield:^4
obviously you'll need to pull in this PR instead.Issues
Pull request checklist