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.
Updated: Comment 0
Problem/Motivation
Proposed resolution
Convert exposed forms to use FormInterface
Remaining tasks
User interface changes
API changes
Comment | File | Size | Author |
---|---|---|---|
#10 | form-2147669-10.patch | 27.05 KB | tim.plunkett |
Comments
Comment #1
dawehnerLet's see what the testbot tells us.
Comment #2
dawehnerA lot of really great points of tim later.
Comment #4
tim.plunkettmergeDeep *always* confuses me. I do this wrong every time.
When you add a check for form errors to the end of testGetCache you see that the whole method was flawed, and was not actually testing the proper behavior. This was exposed by these changes.
Comment #6
dawehnerperfect!
Comment #7
tim.plunkettThe installer is drunk. I manually tested and it failed, and pointed me to what I fixed in the interdiff. I don't know that you could ever have those args be by reference, but it doesn't seem to hurt to remove it now.
Comment #8
tim.plunkettI take it back, not the installer's fault. The mergeDeep is stripping references. Here's an alternate patch.
Comment #9
tim.plunkettI don't want to trigger another retest until those two come back, so just leaving this here for now:
Comment #10
tim.plunkettFinally remembered this issue, whoops.
Comment #11
dawehnerLet's ask damian for a review.
Comment #12
tim.plunkett10: form-2147669-10.patch queued for re-testing.
Comment #13
dawehnerI am fine with that now.
Comment #14
webchickCommitted and pushed to 8.x. Thanks!
Comment #16
cilefen CreditAttribution: cilefen commented