Title says it all, patch attached, part of the meta-issue to make sure all hooks are documented

#675046: Make sure all hooks in D7 have documentation

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dries’s picture

Do we need the || ini_get('allow_url_fopen') in the example? Sounds a litte messy.

jhodgdon’s picture

Status: Needs review » Needs work

A few minor issues with the doc as well:
a)

+ *.  being provided. This array can contain the following keys.

extraneous . at start of that line, and it should end with :

b) I don't think backend is a word. How about back ends?

c) Do you want to explain what a "file transfer back end" is? I didn't find that information in the doc, and have no idea, except that in the @return section it mentioned "mechanisms", which makes more sense to me... But a couple of lines saying "The update system uses file transfer back ends to..." and/or "A file transfer back end is .... For example, ..." might help.

d)

 - settings-form: 

This key is settings_form not settings-form in the code example.

e) Dries's comment -- I think it comes from the real code, but I kind of agree it doesn't illuminate much as an example.

gdd’s picture

Status: Needs work » Needs review
FileSize
1.93 KB

I agree that 'backend' is confusing, but since it is was the code uses, it is what I used. I shouldn't get into a long explanation of it here IMO, the @see references provide far more complete detail for those who need it.

The allow_furl_open in the example was in fact from the original code but I'm happy to pull it out.

Status: Needs review » Needs work

The last submitted patch, 727282.patch, failed testing.

jhodgdon’s picture

How about "file transfer mechanisms" rather than "file transfer backends" for the one-liner doc? That is clearer to me -- and I really want one-liners to be clear. There is a mix of "mechanism" and "backend" in the doc, which I don't like... Stick to one or the other (preferably "mechanism" unless you can demonstrate that "backend" is ubiquitously used in the industry for this purpose -- I have never seen it before and didn't immediately know what it meant). Just because the name of the function uses "backend" doesn't mean that the doc should also use it, if it's not a standard term that everyone would immediately understand.

Also, it looks like this function can return more than one backend/mechanism, so in @return it should say "mechanisms" being provided.

 *   being provided. This array can contain the following keys.

keys. -> keys:

One more very small nitpick:

+ *   - title: Title of this backend to be shown to the end user.
+ *   - class: The name of the PHP class which implements this backend.

Either add "the" before title or remove "the" before name, for parallelism. :)

gdd’s picture

FileSize
1.93 KB

It seems silly to me to be referring to mechanisms when everything in the hook title and the actual sample code refers to backends. For instance the doc would read

 *   - title: Title of the mechanism to be shown to the end user.

whereas in the code sample it is

    $backends['ftp'] = array(
      'title' => t('FTP'),
      'class' => 'FileTransferFTP',
      'settings_form' => 'system_filetransfer_backend_form_ftp',
      'weight' => 0,
    );

IE obviously the title is associated with an array $backends. While I agree that this is a possibly terrible name, I feel like two ways to refer to the same thing may engender more confusion. We should refer to it consistently. So attached is a patch which actually removes all references to mechanism and replaces them with backend, plus the other tweaks requested.

gdd’s picture

Status: Needs work » Needs review
moshe weitzman’s picture

Mechanism is even less clear than backend to me. At least backend implies no UI. Mechanism could be anything.

jhodgdon’s picture

OK... Could we then explain exactly what a "backend" is in the doc? The first time I read through that doc I had zero idea what in the world a "backend" was until I saw in the example that FTP was a backend, and then I had an inkling. Maybe a sentence in the main function doc explaining what a "file transfer backend" is, and what it would be used for?

And is this an industry-standard term? If not, I still think it should be "back ends" not "backends", which is not an English word. Unless you can assure me that this is the standard term in use outside of this Drupal API.

moshe weitzman’s picture

I mean no malice here: http://lmgtfy.com/?q=backend&l=1

jhodgdon’s picture

When I google "file transfer backend" (without the quotes), the first hit's page title is "file transfer back-end" from ubuntu.com.
The next one that's not a forum uses "back end".
There are also some that use "backend" in the first page of results.

I wouldn't say there's any consistency.

jhodgdon’s picture

Meanwhile, whether we use back end, back-end, or backend, I still think the doc could explain what the back-end is going to be used for, what it is, etc. Which the latest proposal doesn't do.

And if you want to standardize on "backend" for Drupal, whatever, though I don't think it's in any way consistent.

BTW, the first 4 hits when I google just "backend" use "back-end", so I'm not sure what you were trying to say there moshe.

jhodgdon’s picture

OK. Let's standardize on backends, I'm convinced enough.

Here's a new patch with a bit more explanation.

gdd’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Great work, folks!

Committed to HEAD.

Status: Fixed » Closed (fixed)

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