The IRC bot complains that the drupal_alter summary for version 6 is not one line.

" drupal_alter: [Documented summary is not one line. This is a bug. File a patch!] => drupal_alter($type, &$data) => http://api.drupal.org/api/function/drupal_alter/6"

Original Summary:
This dispatch function hands off structured Drupal arrays to type-specific *_alter implementations. It ensures a consistent interface for all altering operations.

Possible Patch:
This dispatch function hands off structured Drupal arrays to type-specific *_alter implementations while ensuring a consistent interface for all altering operations.

CommentFileSizeAuthor
#18 drupal-alter2-D6.patch887 bytesrdrh555
#15 drupal-alter-D6.patch860 bytesrdrh555
#6 drupal_docs-294742-6-axyjo.patch491 bytesaxyjo
#2 drupal_docs-294742-2-axyjo.patch442 bytesaxyjo

Comments

gpk’s picture

gpk commented
Project: Documentation » Drupal core
Version: » 7.x-dev
Component: Developer Guide » documentation
Priority: Normal » Minor

The usual approach is to have a short one-liner first (e.g. "Dispatcher for *_alter implementations.") followed by a longer description (which is what we have at present).

This actually needs to be patched in core itself.

Also needs fixing in 6.x.

Can you make a proper patch? http://drupal.org/patch/create

axyjo’s picture

axyjo commented
StatusFileSize
new drupal_docs-294742-2-axyjo.patch442 bytes

Here it is. Can someone just check the one-liner to see if it makes sense?

axyjo’s picture

axyjo commented
Assigned: Unassigned » axyjo
Status: Active » Needs review

Forgot setting changes.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Needs work

I think it should read Dispatch arrays for *_alter implementations.

You need to update your working copy as well. Your 400+ lines off from the HEAD version.

axyjo’s picture

axyjo commented

Whoops.. Diffed it against the wrong copy of Drupal. I'll also update my HEAD, just in case. I'll edit this post when I'm done.

axyjo’s picture

axyjo commented
StatusFileSize
new drupal_docs-294742-6-axyjo.patch491 bytes

Alright. This is against the HEAD of around 5 minutes ago. Should fix the line number issue. And also fixed the message.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Reviewed & tested by the community
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Version: 7.x-dev » 6.x-dev

I actually thought #2 was a bit more clear, so committed that one. Thanks!

Moving back for 6.x.

moshe weitzman’s picture

moshe weitzman commented

Looks to me like we repeat text now about structured arrays ...

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Version: 6.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Needs work

Hm. That's true. But Dispatch arrays for *_alter implementations. is far too short and non-descript to be an adequate summary of the function.

Sounds like the "bigger" part after the one-line summary could use some work, then. In fact, we might want to be more verbose about how this function works since it's the lynchpin of all of our hook_foo_alter() hooks.

axyjo, could you take another stab?

axyjo’s picture

axyjo commented
Status: Needs work » Fixed

This was taken care of in #593522: Upgrade drupal_alter().

gpk’s picture

gpk commented
Version: 7.x-dev » 6.x-dev
Status: Fixed » Patch (to be ported)

OK needs backporting per #593522: Upgrade drupal_alter() then.

axyjo’s picture

axyjo commented

6.x already has a description though.

gpk’s picture

gpk commented

As you said in your original report, there is a description but no one-line summary.

rdrh555’s picture

rdrh555 commented
Assigned: axyjo » rdrh555
Status: Patch (to be ported) » Needs review
StatusFileSize
new drupal-alter-D6.patch860 bytes

Lots of changes in D7, but I think I did what was needed here. Per #11, added (D7) one liner and (D7) descriptive summary (webchick #10).

jhodgdon’s picture

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

This looks fine to me. Although the D7 documentation has been much expanded since the patches above, the expansions appear all to be new functionality added to the function, and they don't apply to D6.

So I think the above patch in #15 is the right thing for D7. Thanks!

jhodgdon’s picture

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

Patch needs to be rerolled from Drupal root.

rdrh555’s picture

rdrh555 commented
Status: Needs work » Needs review
StatusFileSize
new drupal-alter2-D6.patch887 bytes

Wrong Eclipse setting...correct patch:

jhodgdon’s picture

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

Thanks.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Reviewed & tested by the community » Fixed

Thanks, committed.

Status: Fixed » Closed (fixed)

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