Background:
This issue is part of the meta-issue for updating the hook_help texts of the Drupal 8 modules:
#1908570: [meta] Update or create hook_help() texts for D8 core modules

Tasks:
- review / write the hook_help text according to help guidelines http://drupal.org/node/632280
More details are on the meta-issue.

CommentFileSizeAuthor
#16 2103045-remove-lines.patch1.39 KBjhodgdon
#13 interdiff.txt1.31 KBbatigolix
#13 xmlrpc_updated_help-documentation.2103045-12.patch1.44 KBbatigolix
#8 xmlrpc_updated_help-documentation.2103045-8.patch1.43 KBbatigolix
#4 xmlrpc_updated_help-documentation.2103045-4.patch1.46 KBbatigolix
#1 xmlrpc_updated_help-documentation.2103045-1.patch1.29 KBpetrpo
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

petrpo’s picture

petrpo CreditAttribution: petrpo commented
Status: Active » Needs review
FileSize
xmlrpc_updated_help-documentation.2103045-1.patch1.29 KB

Patch corrects docs according to D8 documentaion.

petrpo’s picture

petrpo CreditAttribution: petrpo commented

simpletest.me went ok & qa as well. Ready for comments and reviews.

jhodgdon’s picture

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

This help text does not follow the guidelines/tempate for hook_help() (link is in the issue summary). Thanks for the URL fixes... but it needs more work.

batigolix’s picture

batigolix
Primary language Dutch
Location Utrecht
CreditAttribution: batigolix commented
Status: Needs work » Needs review
FileSize
xmlrpc_updated_help-documentation.2103045-4.patch1.46 KB

Patch:

- adds link to wikipedia
- adds "about" header
- adds link to xml-rpc docs page on d.o.
- brings text in line with guidelines

For the link to xmlrpc.php I followed the example of links to update.php.

The text could still need a bit of re-writing

Status: Needs review » Needs work

The last submitted patch, xmlrpc_updated_help-documentation.2103045-4.patch, failed testing.

jhodgdon’s picture

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

#4: xmlrpc_updated_help-documentation.2103045-4.patch queued for re-testing.

jhodgdon’s picture

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

Thanks, and sorry I didn't review this sooner!

There are a couple of things I think still need to be addressed:

a) I think that we should not put in a link to an English Wikipedia page unless the URL is part of the text that is the first argument with t() - a translator might want to link to a translated version of that page. So instead of using a !name variable substitution, I think you should just put the URL into the text.

b) This is not very great wording: "Pointing an XML-RPC client at...". Maybe "Directing ... to"?

c) The link to the online docs still does not follow our template.

batigolix’s picture

batigolix
Primary language Dutch
Location Utrecht
CreditAttribution: batigolix commented
FileSize
xmlrpc_updated_help-documentation.2103045-8.patch1.43 KB

Patch addresses a) - c) from #7.

Regarding b), I chose a simpeler wording:
"An XML-RPC client can communicate with the site by calling xmlrpc.php"

batigolix’s picture

batigolix
Primary language Dutch
Location Utrecht
CreditAttribution: batigolix commented
Status: Needs work » Needs review

setting status

jhodgdon’s picture

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

When you make a patch that corrects an earlier patch, and interdiff is requested.
https://drupal.org/documentation/git/interdiff
Thanks!

Anyway... I am not sure I agree with the wording "calling (a URL)". I don't think you "call" a URL. You "call" a PHP function, but you don't "call" a URL.

I'm also not sure if it is a good idea to build a URL to xmlrpc.php by using $base_url . '/xmlrpc.php'. Actually, this is totally unnecessary anyway -- this URL is part of the xmlrpc.routing.yml file, so you can use https://drupal.org/node/632280#url-note

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

Just sat down next to Lin Clark for a moment and she said we should say "... make a request to (URL)" not "call (URL)". That is the correct term. Thanks!

batigolix’s picture

batigolix
Primary language Dutch
Location Utrecht
CreditAttribution: batigolix commented

Patch in #13 addresses the points made in #10 and #11

batigolix’s picture

batigolix
Primary language Dutch
Location Utrecht
CreditAttribution: batigolix commented
Issue summary: View changes
Status: Needs work » Needs review
FileSize
xmlrpc_updated_help-documentation.2103045-12.patch1.44 KB
interdiff.txt1.31 KB
petrpo’s picture

petrpo CreditAttribution: petrpo commented

Hi, I tried to execute simpletest.me. I was Ok.

jhodgdon’s picture

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

Thanks! The new patch looks good to me and has been manually tested, so it is ready to go in.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Reviewed & tested by the community » Needs review
FileSize
2103045-remove-lines.patch1.39 KB

Uh oh.

I was going to commit this patch but I noticed an extra space at the end of the 2nd line of xmlrpc_help(), which had to be removed.

So then I noticed tha tglobal $base_url; line at the top, which we do not need.

So I'm removing those two lines from the patch and running a retest just in case. I did not make an interdiff. The patch is the same except the first two lines

  global $base_url;

have been removed from xmlrpc_help().

Assuming the test bot agrees, this should be good to go still...

jhodgdon’s picture

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

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

Thanks again everyone! Committed to 8.x.

Status: Fixed » Closed (fixed)

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