Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Let's not continue to babysit contrib.
Comment | File | Size | Author |
---|---|---|---|
#11 | move-param.patch | 4.23 KB | xjm |
#5 | views-1808810-5.patch | 14.78 KB | tim.plunkett |
#2 | views-1808810-2.patch | 12.64 KB | tim.plunkett |
views-moved.patch | 12.54 KB | tim.plunkett | |
Comments
Comment #1
tim.plunkettComment #2
tim.plunkettJust a quick test.
Comment #3
merlinofchaos CreditAttribution: merlinofchaos commentedThis isn't really babysitting; I think it's a very important feature.
While its primary purpose is version to version upgrading, sometimes tables and fields get renamed/moved, and this is the *only* way to update exported Views to keep them working. Without this, table/field names are basically locked forever.
I would really suggest not removing this.
Comment #5
tim.plunkettWith CMI we'll actually be able to reliably update this kind of stuff, since we don't have to worry about if its in the DB or in files spread around the file system, everything is central.
And since we'll be in core, modules renaming their tables/fields will handle it themselves.
Cleaned up more, I think this is fine.
Comment #6
tim.plunkettThese parts are directly related but not required, so I'm calling them out here. I don't think it's worth a separate follow-up.
Somewhere along the way, 'string' became the original value of this, so if override was taken out this still passed.
These were the only cases that needed this code.
$override is either a string or NULL, no need to assign it above and check it again
yay less procedural code
While removing update() I realized we still have this old code.
Comment #7
dawehnerWell in d7 this was actually mostly used for the upgrade path and for that we will provide a better way.
Do you have any links regarding the automatic changing of config, is there anything concrete planned yet?
Currently it is a string, so this makes sense: 'filter' => array(
'id' => 'string',
),
Does this actually makes sense to be runned via t(), as this is basically something which will never be shown to the enduser. I know this is out of scope, but i'm just wondering.
Especially i guess this is actually broken code?
Comment #8
tim.plunkettI talked to heyrocker, we can simply have a hook_update_N() go through each views.view.*.yml file and update everything :) Yay!
Talked about the other points in IRC. RTBC?
Comment #9
dawehnerWell, i would probably suggest to write some helper api methods then, because we want to make it as easy as possible.
Comment #10
tim.plunketthttp://drupalcode.org/project/views.git/commit/6778f04
I don't know how we would write a generic helper, but when we start to talk about upgrade path I'm sure we'll come up with one.
Comment #11
xjmThe
$move
parameter was still inviews_fetch_data()
. Attached removes that and updates the docs.Comment #12
aspilicious CreditAttribution: aspilicious commentedReviewed!
Comment #13
xjmPushed to 8.x-3.x: http://drupalcode.org/project/views.git/commit/caa8d2c