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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Désiré’s picture

Some more misused @ingroup fixed.

Désiré’s picture

FileSize
1.5 KB

Interdiff for #1

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -doxygen, -docs infrastructure +Needs backport to D7

Looks 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

catch’s picture

Status: Reviewed & tested by the community » Needs review
- * @addtogroup updates-7.x-to-8.x
+ * @addtogroup updates_7_x_to_8_x
  * @{

This 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.

jhodgdon’s picture

Status: Needs review » Needs work

I'd be in favor for changing the group flag to updates_7_8 also. Good idea.

Désiré’s picture

Status: Needs work » Needs review
FileSize
5.69 KB
24.7 KB

Here is the new version.

jhodgdon’s picture

Actually, this group of functions should not even have 7->8 on it at all, according to the description:

 /**
- * @defgroup update-api-7.x-to-8.x Update versions of API functions
+ * @defgroup update_api_7_8 Update versions of API functions
  * @{
  * Functions similar to normal API function but not firing hooks.

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...

jhodgdon’s picture

Status: Needs review » Needs work

oops, I had forgotten to change the status. Let's remove that group/topic.

wulff’s picture

On a related note, the @defgroup in system.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?

wulff’s picture

In 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 @defgroups to @addtogroups.

jhodgdon’s picture

RE #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.

jhodgdon’s picture

Title: Invalid doxygen group names, and misused @ingroup commands » Misused @ingroup commands

The 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.

jhodgdon’s picture

RE #9 - I just filed
#1546618: Duplicate @defgroups in Core

I guess the patch in #10 should go there maybe...

wulff’s picture

Status: Needs work » Needs review
FileSize
14.35 KB

I 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).

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

The 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).

jhodgdon’s picture

Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Thanks! 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).

wulff’s picture

Status: Patch (to be ported) » Needs review
FileSize
17.36 KB

I've taken a stab at a patch for 7.x. It fixes the ingroup/addtogroup problem and adds a few missing periods.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Looks 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.

jhodgdon’s picture

There'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

jhodgdon’s picture

Status: Reviewed & tested by the community » Fixed

Oh wait. That issue is 8.x only, and this is 7.x. So I went ahead and committed this. Thanks!

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

Anonymous’s picture

Issue summary: View changes

Update since the original report was only half correct, based on incorrect information in node/1354