Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim’s picture

Though it should be noted that the D8 doc header is wrong: this is not a form.

jhodgdon’s picture

Title: node_add_page() has no docblock? » node_add_page() documentation is wrong in D8 and missing in D7
Version: 7.x-dev » 8.x-dev
Issue tags: +Needs backport to D7

The D8 header does not say it is a form. It says it is a page callback that presents a form... but you are right, there isn't a form there at all. OK, let's document this function correctly in D8 and then backport this correct doc to D7. Thanks for the report and clarification!

mjonesdinero’s picture

Assigned: Unassigned » mjonesdinero
Status: Active » Needs review
FileSize
477 bytes

assigning to me for further feedback.

attach is a patch for my documentation of the function

@jhodgdon
if feedback is past 6pm Philippines time, will continue to work on this on Monday morning in Philippines time also

Thanks

jhodgdon’s picture

Status: Needs review » Needs work

Good try! But what we need to do is replace the existing line that says "Presents the node add form" with a more accurate description, not add text before-hand. Also, the first line of any function documentation needs to be a one-line sentence of less than 80 characters:
http://drupal.org/node/1354#functions
and specifically for page callbacks:
http://drupal.org/node/1354#menu-callback

mjonesdinero’s picture

Status: Needs work » Needs review
FileSize
380 bytes

update the patch

jhodgdon’s picture

Status: Needs review » Needs work

Thanks for the 2nd try! It's a bit better, but this still isn't quite right:
- Verb tense is wrong: http://drupal.org/node/1354#menu-callback
- The text is still inaccurate. The function normally makes a list of links to add different types of content (if your system has multiple content types), and alteratively goes directly to the node/add/[type] page if you only have one content type. This text does not say that at all, and it needs to.

mjonesdinero’s picture

Status: Needs work » Needs review
FileSize
472 bytes

hope this is okie now..

jhodgdon’s picture

Status: Needs review » Needs work

Much better! Maybe though the first line should say:
Displays a list of add content links for available content types.
I make this suggestion because (a) it's not displaying just a list of the content types, but a list of links to add content, and (b) in the other line you added, they're correctly referred to as "content types" rather than "node types". I'm not sure if this will fit in 80 characters... hopefully?

mjonesdinero’s picture

Status: Needs work » Needs review
FileSize
478 bytes

HI jhodgdon.

Displays a list of add content links for available content types.
i change it to
Displays add content links for available content types.

so that it fit for 80lines..

jhodgdon’s picture

Version: 8.x-dev » 7.x-dev
Status: Needs review » Patch (to be ported)

Great! I love that choice of words. Simple, concise, and gets the point across. :)

I've committed the patch in #9 to Drupal 8.x. Time to backport to 7.x, where the function doesn't even have documentation. Thanks!

tim.plunkett’s picture

Status: Patch (to be ported) » Needs review
FileSize
576 bytes

Rerolled.

cafuego’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me.

jhodgdon’s picture

Thanks! I'll get it committed shortly.

jhodgdon’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 7.x -- thanks all!

Status: Fixed » Closed (fixed)

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

joachim’s picture

Version: 7.x-dev » 6.x-dev
Assigned: mjonesdinero » Unassigned
Status: Closed (fixed) » Patch (to be ported)

Shall we backport this to 6.x too?

jhodgdon’s picture

Sounds like a good idea. The function node_add_page() appears to be completely undocumented in D6.

A direct port of the page doesn't seem like the right thing to do, though, since it looks like in D6 the function always displays a list (even if there is only one type of node the person can add).

webchick’s picture

Version: 7.14 » 6.x-dev
Component: page.module » documentation
webchick’s picture

Title: captcha not working in contact form » node_add_page() documentation is wrong in D8 and missing in D7
marcin.wosinek’s picture

Backport to 6.

David_Rothstein’s picture

Status: Patch (to be ported) » Needs review
jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Thanks! The patch looks good for D6. By the way, you need to set the issue status to "needs review" when you upload a patch. :)

Status: Reviewed & tested by the community » Needs work

The last submitted patch, core-node_add_page doc backport-1664070-11.patch, failed testing.

marcin.wosinek’s picture

Status: Needs work » Needs review
FileSize
473 bytes

File name fixed.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

That previous error looks like a test bot glitch... Still RTBC.

jhodgdon’s picture

Oh I see, the file name had spaces in it. Ugh.

jhodgdon’s picture

Status: Reviewed & tested by the community » Fixed

Thanks again -- committed to 6.x.

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