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.
Part of meta-issue #1856630: [Change notice] [META] Rename Views methods to core standards
Comment | File | Size | Author |
---|---|---|---|
#29 | drupal-viewsRename-add_field-2002144-29.patch | 18.57 KB | heddn |
#25 | drupal-2002144-views-addField-25.patch | 17.09 KB | drupalway |
#17 | 2002144-17-rename-add_field.patch | 17.21 KB | elvis2 |
#15 | drupal-2002144-views-addField-15.patch | 18.1 KB | zschmid |
#10 | drupal-2002144-views-addField-6.patch | 16.8 KB | rdrh555 |
Comments
Comment #1
phenaproximaComment #2
phenaproximaComment #3
phenaproximaComment #4
dawehnerThanks for this big conversion.
This should be \Drupal\views\Plugin\views\query\Sql::addField
Comment #5
rdrh555 CreditAttribution: rdrh555 commentedComment #6
sassafrass CreditAttribution: sassafrass commentedApplied patch successfully and did some cursory testing of addField function.
Comment #7
oenie CreditAttribution: oenie commentedAdd public access modifier in front of the method to adher to the new OOP standards.
Comment #8
connorwk CreditAttribution: connorwk commentedComment #9
connorwk CreditAttribution: connorwk commentedFixed what dawehner and oenie pointed out.
Comment #10
rdrh555 CreditAttribution: rdrh555 commentedAfter a search, all remaining 'add_fields' are subsets of other function names. Made @dawehner's change.
Comment #11
rdrh555 CreditAttribution: rdrh555 commentedDisregard #10 patch; this was unassigned from me while I was working on it (#6), not quite sure why...
@connork didn't mean to change assigned, please assign back to yourself.
Comment #12
aspilicious CreditAttribution: aspilicious commentedLooks ok to me
Comment #13
oenie CreditAttribution: oenie commentedHate to kick this back again, but there's still a public access modifier missing.
Comment #14
aspilicious CreditAttribution: aspilicious commentedwhooops, thnx oenie
Comment #15
zschmid CreditAttribution: zschmid commentedadded public access modifier
Comment #16
oenie CreditAttribution: oenie commentedLooks good to me, apart from a minor comment problem, still referencing the old method:
core/modules/views/lib/Drupal/views/Plugin/views/argument/ArgumentPluginBase.php, line 799:
* - add_field: add a 'num_nodes' field for the count. Usually it will
Comment #17
elvis2 CreditAttribution: elvis2 commentedRe-roll. Fixed a few things.
Comment #18
oenie CreditAttribution: oenie commentedLooks go to me now.
You just nicely pointed out another problem that i might not have spotted in other reviews though (references in the comments) :/
But this one looks fine.
Comment #19
catch#17: 2002144-17-rename-add_field.patch queued for re-testing.
Comment #21
dawehner#17: 2002144-17-rename-add_field.patch queued for re-testing.
Comment #23
elvis2 CreditAttribution: elvis2 commentedLooks like this needs a re-roll due to the updated code.
Comment #24
drupalway CreditAttribution: drupalway commentedWe are working today with this issue during Code Sprint UA
Comment #25
drupalway CreditAttribution: drupalway commentedRe-roll. Please review.
Comment #26
podarok#25 looks good for me
if bot happy - rtbc!
Comment #28
star-szrPatch in #25 is p0 formatted, should be p1 formatted.
See https://drupal.org/node/707484 for more information.
We'll need to create a new patch file based on #25 by applying the patch locally with
patch -p0 > patchname.patch
and then generating a new patch file. @drupalway or someone else?Comment #29
heddnLet's see if the testbot likes us.
Comment #30
dawehnerPerfect!
Comment #31
oenie CreditAttribution: oenie commentedLooks good to me !
Comment #32
alexpottCommitted ba4d4a8 and pushed to 8.x. Thanks!