The attach patch corrects the out-dated documentation, and tries to be pretty clear about what's happening, without going into the fact that in PHP5 the handle to the object resource is what's actually handed around.

CommentFileSizeAuthor
#6 1003958-6-remove-reference-reference-in-_node_save_revision-docs.patch1018 bytesmlncn
#3 1003958-3-remove-reference-reference-in-_node_save_revision-docs.patch1.02 KBmlncn
#1 1003958-1-remove-reference-reference-in-_node_save_revision-docs.patch1.22 KBmlncn

Comments

mlncn’s picture

mlncn commented
Status: Active » Needs review
StatusFileSize
new 1003958-1-remove-reference-reference-in-_node_save_revision-docs.patch1.22 KB

ahem. The aforementioned "attached patch".

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Component: node.module » documentation
Status: Needs review » Needs work

I think this text is awkward and I wish we could come up with a better way of saying it without people complaining we are telling people about basic PHP 5 syntax.

There is also a space at the end of one of the lines in your patch, which needs to be removed.

It would also be better to have this in the documentation component, or at least tagged with "needs documentation", because it is a purely doc patch. What is the "dgd7" tag anyway?

mlncn’s picture

mlncn commented
StatusFileSize
new 1003958-3-remove-reference-reference-in-_node_save_revision-docs.patch1.02 KB

Removed not just the space but the extra line of text causing it... Now communicated what actually matters, where the revision ID is available, directly.

I actually like it now, and again, it has the benefit of not being wrong.

("dgd7" is issues that i've come across in the course of work related to Definitive Guide to Drupal 7- if miscellaneous tags are confusing i apologize; i think webchick decided not to use a usingdrupal tag for issues she found revising Using Drupal but i wish she had; i'd love to know / see the list. Speaking of the book, though, jhodgdon... i've been bugging arienk but i've been meaning to ask you: what do you want tens of thousands of book-buyers (and/or torrent downloaders) to know about documentation, as developers or just anyone seeking to contribute to Drupal? hit me in IRC or e-mail!)

mlncn’s picture

mlncn commented
Status: Needs work » Needs review
jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Needs work

I like the docblock line just fine.

I'm not too thrilled about:

+  // Have node object still show node owner's uid, not revision author's.
   $node->uid = $temp_uid;

I don't think we have a concept of the "node owner". How about saying "original node author" instead?

mlncn’s picture

mlncn commented
StatusFileSize
new 1003958-6-remove-reference-reference-in-_node_save_revision-docs.patch1018 bytes

Good point! Node author is what is used in core documentation, and node owner does not appear at all. And node author is much less capitalistic! Corrected. On second thought rerolled again and did not include "original" because the node author can change, the distinction that matters is between revision and node proper.

mlncn’s picture

mlncn commented
Status: Needs work » Needs review
jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

This looks fine to me. Thanks!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 1003958-6-remove-reference-reference-in-_node_save_revision-docs.patch, failed testing.

mlncn’s picture

mlncn commented
Status: Needs work » Reviewed & tested by the community

(This documentation patch is still RTBC, whatever the testing bot thinks about the MySQL environment.)

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

I filed this issue regarding the randomly failing test (you are right there is no way this patch cused that failure):
#1010136: Prevent drupalGet / checkForMetaRefresh from recursing too deep

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Committed to HEAD. Thanks!

Status: Fixed » Closed (fixed)
Issue tags: -dgd7

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

Issue tags: +dgd7

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.