API page: https://api.drupal.org/api/drupal/core%21modules%21hal%21hal.install/8.3.x
This file is defining a topic for updates-8.2.x-to-8.3.x using a @defgroup documentation block.
However, this topic/group has already been defined in serialization.install. It should only be defined once. Subsequent uses should be using @addtogroup.
See
https://www.drupal.org/docs/develop/coding-standards/api-documentation-a...
for documentation on how these tags work.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | interdiff-2-7.txt | 335 bytes | chiranjeeb2410 |
| #7 | hal.install-uses-defgroup-instead-of-addtogroup-2858598-7.patch | 528 bytes | danielnv18 |
| #2 | hal.install-uses-defgroup-instead-of-addtogroup-2858598-1.patch | 549 bytes | danielnv18 |
Comments
Comment #2
danielnv18Comment #3
danielnv18Comment #4
jhodgdonPlease read the documentation linked in the summary for @addtogroup. This isn't quite right. Thanks!
Comment #5
chiranjeeb2410 commented@danielnv18,
We only define each group identifier once with a @defgroup tag. To add more functions to the group present in a separate file or location, use either @addtogroup or @ingroup.
Comment #6
jhodgdon@chiranjeeb2410 -- the patch already does that. The problem is the formatting of the @addtogroup tag. This is why I asked @danilenv18 to please read the documentation, which has an example of the formatting.
Comment #7
danielnv18@jhodgdon thanks for the review. If I understood the documentation correctly this patch should do the work
Comment #8
danielnv18Comment #9
jhodgdonyes, that's it exactly! Thanks!
Comment #10
alexpott@jhodgdon isn't there are bigger issue of where these update groups should be defined - like the module that adds the first update hook doesn't seem right? I think before we should have an issue to go through an check the update hooks and defgroups addtogroup stuff and come up with a procedure to add new groups as we open new branches.
Comment #11
chiranjeeb2410 commented@danielnv18,
+1 on #7. Patch applies cleanly. Uploading interdiff, just to make things clear.
Comment #12
danielnv18I changed the Drupal version from 8.3.x to 8.4.x since Drupal 8.3 is in RC phase but I might be wrong
Comment #13
jhodgdon@alexpott: There is a question in my mind whether there even needs to be a group/topic for these. A group should only be defined if people would really want to look at a page that lists all the grouped functions. Is that really the case for update functions at all? You can find them easily by a search, such as:
https://api.drupal.org/api/drupal/8.3.x/search/_update_82
And by the way, if you click through to
https://api.drupal.org/api/drupal/core%21modules%21path%21path.install/f...
you will notice that there is no topic listed there. The reason is that path.install used
@addtogroup updates-8.2.0
whereas the defgroup was given the ID updates-8.1.x-to-8.2.x
So. This illustrates the problem. To me, it seems like we should just take out all the @defgroup and @addtogroup for update functions entirely, and let people search by function name, which is more reliable as far as finding the actual functions for a particular version. Plus, easier to maintain. That would be my suggestion.
Comment #14
alexpott@jhodgdon that sounds really sensible - far less to get wrong. Let's fix this here because we're unlikely to get this agreed for 8.3.0. I've filed #2860096: Remove api doc groups for updates eg. updates-8.2.x-to-8.3.x.
Committed and pushed 983612d to 8.4.x and ca17ab8 to 8.3.x. Thanks!