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.
Comment | File | Size | Author |
---|---|---|---|
#26 | field-convert-theme-2009008-26.patch | 6.54 KB | pplantinga |
#21 | interdiff-2009008.txt | 657 bytes | benjifisher |
#21 | field-convert-theme-2009008-21.patch | 6.55 KB | benjifisher |
#18 | drupal-remove_theme-2009008-18.patch | 19.35 KB | patoshi |
#12 | field-convert-theme-2009008-12.patch | 6.59 KB | pplantinga |
Comments
Comment #1
thedavidmeister CreditAttribution: thedavidmeister commentedComment #2
thedavidmeister CreditAttribution: thedavidmeister commentedThis may not be as novice as I thought because of:
In FieldPluginBase.php
Comment #3
InternetDevels CreditAttribution: InternetDevels commentedWe are working today with this issue during Code Sprint UA.
Comment #4
InternetDevels CreditAttribution: InternetDevels commentedPatch attached.
Comment #5
InternetDevels CreditAttribution: InternetDevels commentedThis file belongs to the Views module, shouldn't it be replaced in the separate issue?
Comment #7
thedavidmeister CreditAttribution: thedavidmeister commentedThis might be stuck on #1828536: Rename 'type' variable of theme_item_list() to 'list_type'.
Comment #8
star-szr#1828536: Rename 'type' variable of theme_item_list() to 'list_type' is in so maybe we can work on this again.
Comment #9
star-szr…and we can start with a reroll.
Comment #10
pplantinga CreditAttribution: pplantinga commentedFrom scratch.
Comment #11
star-szrThanks @pplantinga, that's looking pretty good I'd say! Found a couple small tweaks.
Needs a trailing comma per http://drupal.org/coding-standards#array.
Same thing here, but I would probably collapse this into one line in which case the trailing comma should not be added.
Nice :) doesn't need tweaking I just like the simplification here.
Comment #12
pplantinga CreditAttribution: pplantinga commentedCool, thanks!
Here's the updated patch.
Comment #14
star-szr#12: field-convert-theme-2009008-12.patch queued for re-testing.
Comment #15
heddn#12 looks good.
Comment #16
YesCT CreditAttribution: YesCT commentedThis issue was RTBC and passing tests on July 1, the beginning of API freeze.
Comment #17
alexpottIt looks like we passing in something different... $variables['element'] and not $variables... why? This appears to be totally unused in
theme_form_required_marker()
Comment #18
patoshi CreditAttribution: patoshi commentedupdated with removed element key.
Comment #20
benjifisherI agree with #17. Since
theme_form_required_marker()
ignores its argument, the original version oftheme_field_multiple_value_form()
might as well calltheme('form_required_marker', array())
and then according to the instructions in the meta-issue #2006152: [meta] Don't call theme() directly anywhere outside drupal_render() we would convert this to(eliminating the
'#element'
key) and then usedrupal_render($form_required_marker)
Comment #21
benjifisherI reviewed the whole patch in #12 and made the change I suggested in #20. I have attached a new patch and an interdiff compared to the patch in #12.
The patch in #18 changed the value of
$form_required_marker = array('#element')
; I entirely remove the value. The patch in #18 also changed core/modules/file/file.field.inc. I think that was an accident.Comment #22
pplantinga CreditAttribution: pplantinga commentedComment #23
benjifisher@pplantinga:
Thanks for changing the status! The test bot seems to approve.
Comment #24
pplantinga CreditAttribution: pplantinga commentedNo idea why I added that snippet. Thanks for removing it.
Looks good to me... +1 (I'll let someone else mark it as RTBC since I wrote most of the patch).
Comment #25
thedavidmeister CreditAttribution: thedavidmeister commentedShould probably be a single line array since it is only a single value.
Other than that, this seems fine to me.
Comment #26
pplantinga CreditAttribution: pplantinga commentedHere.
Comment #27
pplantinga CreditAttribution: pplantinga commentedComment #28
benjifisherAm I disqualified from marking it RTBC since I contributed the patch in #21? As @pplantinga said in #24, he did almost all the work.
I checked that the only difference between #21 and #26 is the change requested in #25.
+1 for RTBC.
Comment #29
thedavidmeister CreditAttribution: thedavidmeister commentedseems fine to me
Comment #30
alexpottCommitted 2a1e147 and pushed to 8.x. Thanks!