API page: http://api.drupal.org/api/drupal/includes--form.inc/function/drupal_retr...

The function contain the following comment:

<?php
 
// $menu_get_item() is not available at installation time.
?>

$menu_get_item() is really the menu_get_item() function, but the comment seems referring to the function whose name is contained in the variable $menu_get_item.

Files: 
CommentFileSizeAuthor
#15 1204784-includes-form-15.patch863 bytesbdgreen
PASSED: [[SimpleTest]]: [MySQL] 40,342 pass(es).
[ View ]
#11 1204784-includes-form-11.patch884 bytesKevin Morse
PASSED: [[SimpleTest]]: [MySQL] 55,574 pass(es).
[ View ]
#9 1204784-includes-form.patch886 bytesKevin Morse
PASSED: [[SimpleTest]]: [MySQL] 55,899 pass(es).
[ View ]
#1 drupal_retrieve_form_doc.patch661 bytesdroplet
PASSED: [[SimpleTest]]: [MySQL] 32,991 pass(es).
[ View ]

Comments

Version:7.x-dev» 8.x-dev
Status:Active» Needs review
Issue tags:+Quick fix
StatusFileSize
new661 bytes
PASSED: [[SimpleTest]]: [MySQL] 32,991 pass(es).
[ View ]

The code right after that is checking for maintenance mode, not "installation time". So is the new comment still a bit misleading too?

The constant is used both in install.php, as define('MAINTENANCE_MODE', 'install'), and in update.php, as define('MAINTENANCE_MODE', 'update').
drupal_retrieve_form() is called by drupal_build_form(), which is then called by install_run_task(); also update_selection_page() indirectly calls drupal_retrieve_form(). The difference is that in the latter case, Drupal is fully bootstrapped, while in the first case, Drupal is not full bootstrapped.

Finding out that is not that immediate; maybe the comment should be less "cryptic" about that point.

Title:Documentation problem with drupal_retrieve_form()drupal_retrieve_form() has wrong code comment
Priority:Normal» Minor

Well, the check is just defined('MAINT...') -- it isn't checking what the value is. So it seems like if we're fixing the comment, it should reflect what the code does?

Changing tags so webchick will get it displayed when searching for "needs backport to D7."

#1: drupal_retrieve_form_doc.patch queued for re-testing.

Status:Needs review» Needs work

patch actually is needs work, see comments above.

Issue tags:+Novice

Sounds like an good first issue for someone with good English

Status:Needs work» Needs review
StatusFileSize
new886 bytes
PASSED: [[SimpleTest]]: [MySQL] 55,899 pass(es).
[ View ]

Figured I'd give this a try... Not sure if I fully understand what's going on but we shall see.

Status:Needs review» Needs work

#9: Looks good, but can you please remove extra spaces in the emd of comment lines?

Status:Needs work» Needs review
StatusFileSize
new884 bytes
PASSED: [[SimpleTest]]: [MySQL] 55,574 pass(es).
[ View ]

Try this...

Looks fine for me, let's see if that sounds clear enough.

Assigned:Unassigned» jhodgdon
Status:Needs review» Reviewed & tested by the community

Thanks! This looks fine to me to. I'll get it committed soon, unless someone else beats me to it.

Version:8.x-dev» 7.x-dev
Assigned:jhodgdon» Unassigned
Status:Reviewed & tested by the community» Patch (to be ported)

Thanks again! Committed to 8.x. The patch does not apply to 7.x and needs a reroll.

Status:Patch (to be ported)» Needs review
StatusFileSize
new863 bytes
PASSED: [[SimpleTest]]: [MySQL] 40,342 pass(es).
[ View ]

Patch rerolled for 7.x

Status:Needs review» Needs work
Issue tags:-needs backport to D6, -Quick fix, -Novice, -needs backport to D7

The last submitted patch, 1204784-includes-form-15.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+needs backport to D6, +Quick fix, +Novice, +needs backport to D7

#15: 1204784-includes-form-15.patch queued for re-testing.

Status:Needs review» Reviewed & tested by the community

Testbot is happy, and change matches D8 version

Status:Reviewed & tested by the community» Needs review

Thanks @valthebald ;)

Similar comments do not appear near function drupal_retrieve_form in D6 form.inc so haven't rolled a patch for D6. OK?

Status:Needs review» Reviewed & tested by the community
Issue tags:-needs backport to D6, -needs backport to D7

Right.
Removind 'Needs backport...' tags (as far as we're already at 7.x)

Assigned:Unassigned» jhodgdon

Agreed, the patch is not needed for Drupal 6.x. I'll get the 7.x patch committed soon. Thanks!

Assigned:jhodgdon» Unassigned
Status:Reviewed & tested by the community» Fixed

Thanks again! Committed to 7.x.

Automatically closed -- issue fixed for 2 weeks with no activity.