Problem/Motivation
Some places in the documentation use "@ingroup" instead of "@addtogroup"
If you have functions that need to be added to a group whose definition is not right above the functions, there are two ways to do it:
If there is just one function to be added, add
* @ingroup group_identifier
to the documentation block for the function.
If there are multiple functions to be added, put/** * @addtogroup group_identifier * @{ */
before the functions, and
/** * @} End of "addtogroup group_identifier". */
after the group of functions.
See more in coding standards documentation: http://drupal.org/node/1354#groups
Proposed resolution
Replace the wrong @ingropup commands with @addtogroup.
And fix the comments after the @defgroup and @addgroup command closings.
Remaining tasks
Review needed, and backporting to D7.
User interface changes
No interface changes.
API changes
Just documentation changes.
Comments
Comment #1
Désiré CreditAttribution: Désiré commentedSome more misused @ingroup fixed.
Comment #2
Désiré CreditAttribution: Désiré commentedInterdiff for #1
Comment #3
jhodgdonLooks good! Thanks for noticing and fixing this problem. I think we should backport this to D7 too (will require a somewhat different patch).
For future reference, please read the issue tag guidelines (avoid adding non-standard tags to issues):
http://drupal.org/node/1023102
Comment #4
catchThis is much less readable, can we look at trying to work around this a different way maybe? I think I'd prefer updates_7_8 for example although better ideas welcome.
Comment #5
jhodgdonI'd be in favor for changing the group flag to updates_7_8 also. Good idea.
Comment #6
Désiré CreditAttribution: Désiré commentedHere is the new version.
Comment #7
jhodgdonActually, this group of functions should not even have 7->8 on it at all, according to the description:
http://api.drupal.org/api/drupal/core--includes--update.inc/group/update...
Hmmm... That group/topic is actually completely empty and should be removed, I think? There's nothing in it for Drupal 7 either...
http://api.drupal.org/api/drupal/includes--update.inc/group/update-api-6...
Comment #8
jhodgdonoops, I had forgotten to change the status. Let's remove that group/topic.
Comment #9
wulff CreditAttribution: wulff commentedOn a related note, the
@defgroup
insystem.install
should be changed to@addtogroup
to maintain consistency with the other install files (e.g. comment, node, user, etc.).Should I work this change into the patch in #6?
Comment #10
wulff CreditAttribution: wulff commentedIn D7, the use of
@defgroup
instead of@addtogroup
causes database errors in the API module (see #1541318: Database error in api_shutdown() function).The attached patch changes the offending
@defgroup
s to@addtogroup
s.Comment #11
jhodgdonRE #9 - please file a separate issue for that problem, as it is unrelated to this issue.
And regarding the original issue report here, actually the documentation about what group names can contain is incorrect, since we allow hyphens and . now in the API module. I just fixed up the docs. Sorry I didn't remember this earlier... I'll edit the issue summary.
Comment #12
jhodgdonThe issue summary is fixed. The patch in #6 needs a reroll just to fix the ingroup/addtogroup problems, and let's just leave the group names as they were, as they are not causing any API module parsing problems.
Comment #13
jhodgdonRE #9 - I just filed
#1546618: Duplicate @defgroups in Core
I guess the patch in #10 should go there maybe...
Comment #14
wulff CreditAttribution: wulff commentedI have rerolled the patch from #6. It only fixes the ingroup/addtogroup problem and some minor style issues (based on http://drupal.org/node/1354#groups).
Comment #15
jhodgdonThe patch looks good, thanks!
Since it hits quite a few files, I need to schedule its commit. So, unless I hear otherwise (or unless there are any avoid-conflict-tagged patches in the queue at the time that will conflict), I plan to commit this patch to 8.x on Wednesday, May 16.
After that (please wait until then!!) it will need a re-roll for 7.x (with slight modifications, because I'm sure the 7-8 update group will be 6-7 instead, for instance).
Comment #16
jhodgdonThanks! Committed to 8.x. We now need to do something similar for 7.x (a straight reroll won't be possible due to some different group names).
Comment #17
wulff CreditAttribution: wulff commentedI've taken a stab at a patch for 7.x. It fixes the ingroup/addtogroup problem and adds a few missing periods.
Comment #18
jhodgdonLooks good, thanks! Like the 8.x patch above, this one touches a lot of files. So I'll let it sit for a few days, and commit it Tue May 22 unless someone objects or there is a patch tagged "avoid commit conflicts" that conflicts with it.
Comment #19
jhodgdonThere's a large patch with the "avoid commit conflicts" tag right now, so I'm postponing committing this until that gets in, just in case.
#1470824: XML encoder can only handle a small subset of PHP arrays, so switch to YAML
Comment #20
jhodgdonOh wait. That issue is 8.x only, and this is 7.x. So I went ahead and committed this. Thanks!
Comment #21.0
(not verified) CreditAttribution: commentedUpdate since the original report was only half correct, based on incorrect information in node/1354