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.
In core/modules/system/tests/modules/ajax_forms_test/ajax_forms_test.module
there are two docblocks with an @return
directive of unknown_type
. To be consistent with the rest of core, it should be mixed
. Patch to follow.
Comment | File | Size | Author |
---|---|---|---|
#10 | system-return-values-1754712-9.patch | 685 bytes | TravisCarden |
#5 | system-return-values-1754712-5.patch | 785 bytes | pingers |
#2 | system-return-values-1754712-2.patch | 796 bytes | Bußmeyer |
#1 | system-return-mixed-1754712-1.patch | 814 bytes | TravisCarden |
Comments
Comment #1
TravisCarden CreditAttribution: TravisCarden commentedComment #2
Bußmeyer CreditAttribution: Bußmeyer commentedHey TravisCarden,
Thank you for your patch. I have reviewed them.
function ajax_forms_test_menu() {
is an implemetation of a hook. The params and return values should not be included. See: http://drupal.org/node/1354#hookimpl
This function defines a form, right? The return value is an array isn't it?
I created a new patch.
Regards,
Thomas
Comment #3
mcjim CreditAttribution: mcjim commentedThanks both for your patches.
Bußmeyer is right, implementations of a hook don't include the params and return values.
Form generating functions don't, either (well, not for $form and $form_state or the return value), so we can have just:
According to http://drupal.org/node/1354#forms, you can also add
@ingroup forms
, which groups all forms into the Form Builder topic on api.drupal.org.So… almost there :-)
Comment #4
TravisCarden CreditAttribution: TravisCarden commentedThank you both. @mcjim: This is just a test module. Do we
@ingroup forms
functions in tests?Comment #5
pingers CreditAttribution: pingers commentedFrom /core/modules
Seems a bit inconsistent re: @ingroup. Re-rolled without the change.
Comment #6
pingers CreditAttribution: pingers commentedDoh. Test it!
Comment #7
mcjim CreditAttribution: mcjim commentedInteresting observation about use of @ingroup, pingers.
Looking at core/modules/system/tests/form_test, the term "Form constructor" is used 10 times but "@ingroup forms" only 5. But that's an inconsistency to be discussed in another issue.
I think this is good to go.
Comment #8
webchickThis looks right to me!
Committed and pushed to 8.x. Thanks!
Does this need backport to D7 as well?
Comment #9
jhodgdonLooks like it does need a backport. The file is
modules/simpletest/tests/ajax_forms_test.module
Comment #10
TravisCarden CreditAttribution: TravisCarden commentedComment #11
TravisCarden CreditAttribution: TravisCarden commentedA straight backport of a trivial patch can be RTBC'ed once it passes automated testing, no?
Comment #12
webchickLooks the same to me. :)
Committed and pushed to 7.x. Thanks!