node_view still mentioned in API docs

Also, the upgrade guide does not appear to mention node_view http://drupal.org/node/224333 -- I plan to fix that too

Apparently in HEAD revision 1.1019 (see http://drupal.org/node/351235), a decision was made to replace node_view() with node_build(), more or less. node_build() returns an array rather than theming it as node_view() did. That patch was in order to "keep things as structured arrays as long as possible", i.e. theme later.

So there are 3 places in the code comments that still mention node_view(). They need to be reviewed to see if node_build applies or not. And we probably need a note in the migration guide to say something about this change.

Here's a patch for the mention in common.inc. This is a comment on drupal_render(), which should no longer mention node_view as an example of a function that should call drupal_render() [node_build() does NOT call drupal_render, as it is keeping things as an array]

This second patch is for the node.api.php file (documentation for the hooks).

First, in hook_nodeapi_alter -- this hook is now not called from node_view or node_build, but only in node_feed, so the doc needs to reflect that.

Second, in hook_nodeapi_view -- just needs to point to node_build rather than node_view.

Please commit both patches above (assuming they are favorably reviewed).

Sorry I'm going to be super nit-picky, but there are trailing space(s) in the patch that need to be removed.

Sorry about that!
Here's a new version of the "b" patch. I think "a" is fine.

I don't have permission to edit the migration guide to add a note that node_view is gone. I filed a doc issue on that: http://drupal.org/node/394308

The patches from #4, and #9 both appear ready to commit. They apply with offset, but no big deal there.

Did a quick search for any other mentions of node_view that should be covered by this but didn't find anything.

Someone with appropriate access still needs to include the documentation from #394308: 6.x to 7.x module migration guide needs additional section

First of all, thank you so much for being conscientious about these docs. It's vitally important that our docs remain up to date in order to not slow down contributors, and it's an area we don't have enough people looking after. YAY!

A couple minor things with this patch. I could just fix 'em, but since this looks like one of your first core patches, I want to try and help give you some pointers. :)

1. Rather than uploading multiple issues to the queue, please combine all of your patches in one. You can do this by changing both files and then running the command cvs diff -up > whatever.patch from the root directory. Note that the -up is important because it'll not only do the patch in "unified" format (with the +/- lines) but also will give some hint as to what function the code you're changing is in. (which tends to fail miserably with PHPDoc changes like this, but is a good habit to get into nonetheless.) If you have questions on any of this, please don't hesitate to drop by IRC and ask someone. We can walk you through anything you're having trouble with!

2. When you remove something from a comment line (as in patch a), please adjust the surrounding text so that comments continue to wrap at 80 characters. Otherwise it looks a bit jarring when one line is way shorter than another for no obvious reason.

3. But rather than simply removing node_view() in a), how about we mention somewhere else that drupal_render() is called from? By expanding out the "56 functions that call drupal_render()" on http://api.drupal.org/api/function/drupal_render/7, at least half of them are theme functions. So let's change "or node_view" to "or theme functions" perhaps.

4. As long as you're touching the PHPDoc for hook_node_view anyway, how about wrap that " * will be called after hook_view(). The structure of $node->content is a renderable" line at 80 characters as well?

Thanks, Jennifer!

Oh, and additionally:

  * @return
  *   None.

If a function doesn't return anything, it's fine to just omit the @return thing.

OK, here is a new patch.

I combined (a) and (b) patches above into one patch. I don't have the CVS command on this machine -- using Eclipse -- so I can't do a cvs diff -up... hopefully this format is OK. It is a unified diff, but it doesn't do the "-p" stuff to include the function name. Eclipse doesn't appear to have any option to set this up. Maybe I'll get a command line CVS for this machine (via Cygwin probably)... or use my other machine to make patches (but it's a pain, my linux machine is a laptop).

Anyway. This patch also removes a bunch of @return: None things in a bunch of documentation, and fixes the wrapping for the function docs that are being edited.

Stupid thing keeps deleting my comments!!!

Please use the patch in #17 not #16. Spaces were at the ends of lines.

#16: You should just be able to: team -> update. It will merge changes from core then role the patch.

The issue is not how to make a merged patch. I didn't have any trouble doing that in Eclipse.

The issue is the -p flag on cvs diff (the -p flag tries to make sure to include the function definition, so you can see what function is being patched). Eclipse doesn't (apparently) use that flag when it creates a patch.

FYI, #367214: hook_node_alter almost disappeared ?

Here is a new patch. Due to #449718: node_feed() is using the old node building API (see also #367214: hook_node_alter almost disappeared ?), hook_node_alter() went away, and my last patch would not apply. So this patch became even simpler.

Why are there unrelated common.inc changes in the patch?

Well, I could take those common.inc changes out. I was just trying to help with comment #14 above; noticed several other places where these things were there. Is that bad form?

Only issue with the common.inc changes is there's a lot of changes to comments which are just whitespace - makes the patch harder to review because you have to read through line by line to see if any text changed. It's not bad as such, but it's much quicker to review two small and focused patches than one bigger one.

http://webchick.net/please-stop-eating-baby-kittens

Point taken.

Here's a new patch, with only the changes specific to this issue.

Just as a comment, I was specifically asked to fix the whitespace/wrapping lines in the functions whose documentation was being edited. See comments above.

Or maybe I misinterpreted Webchick's request, and fixed wrapping in too much of that particular function doc?

Looks good. Committed to CVS HEAD. Feel free to follow-up with more patches as necessary.