Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
We should have more setters/getters for the important stuff.
Postponing until we stop moving everything around :)
Comment | File | Size | Author |
---|---|---|---|
#3 | views-1792844-3.patch | 89.93 KB | tim.plunkett |
Comments
Comment #1
xjmComment #2
damiankloip CreditAttribution: damiankloip commentedWe talked about this on IRC, but for the benefit of this issue;
This could be pretty tricky at the moment, as the config entity controller uses Reflection to save all the public properties from ViewStorage into config. This was one of the reasons we made $executable protected when we split the classes and add a getExecutable to access it.
Comment #3
tim.plunkettThis blocks #1798574: Refactor Views UI to be a form controller.
Because CMI can only write public properties to YAML files, we can't actually make these protected.
In the meantime, we can use setters and getters everywhere.
Comment #4
dawehnerGreat cleanup! I kind of dislike that we have to first store a variable before using !empty but that's life.
Comment #5
webchickI was confused about this patch (a little bit more context in these patches for people no on the VDC team would be helpful), but tim explained in irc that Views is trying to move away from being able to use $views->foo directly, both for proper refactoring of Views UI, as well as for translatable stuff, etc.
I asked why it was onyl covering a couple of things ('name', base_name') rather than everything and was told that it's because it's focused only on the public properties of a view for now. Further clean-up will happen in subsequent patches.
Since this is just basically a find/replace job, looks good here. I confess I didn't check each hunk individually but I'm guessing any issues will expose themselves pretty fast. ;)
Committed and pushed to 8.x (minus a hunk about the code that was committed as part of #1810826: Remove obsolete form code from views_ui/admin.inc). Thanks!
Comment #7
xjmComment #8
xjmComment #9
Chris Matthews CreditAttribution: Chris Matthews as a volunteer and at City of Oaks Design commentedFor more information as to why this issue was moved to the Drupal core project, please see issue #3030347: Plan to clean process issue queue
Comment #10
Chris Matthews CreditAttribution: Chris Matthews as a volunteer and at City of Oaks Design commentedMoving back to the contributed Views issue queue and closing as outdated per https://www.drupal.org/project/views/issues/3030347#comment-13023447