Problem/Motivation

The @addtogroup updates-8.2.x-to-8.3.x and corresponding @defgroup statements are really hard to maintain and don't add much. To quote @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.

Proposed resolution

Remove all of the @defgroup and @addtogroup statements for groups of update functions. And don't add new ones.

Remaining tasks

User interface changes

None

API changes

None

Data model changes

None

CommentFileSizeAuthor
#10 2860096-8.3.patch21.55 KBalexpott
#5 remove_api_doc_groups-2860096-5.patch24.9 KBGoZ
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Title: hal.install has @defgroup instead of @addtogroup » Remove api doc groups for updates eg. updates-8.2.x-to-8.3.x
Issue summary: View changes
jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Issue summary: View changes

Just as a note, you'd want to remove both the @defgroup statements, which are sprinkled around various *.install files, and the @addtogroup statements, which are sprinkled in other *.install files. Making this minor update to the summary. Obviously, I'm +1 on the idea.

Is there a new API documentation maintainer to weigh in? :)

GoZ’s picture

GoZ CreditAttribution: GoZ at Centarro commented
Assigned: Unassigned » GoZ

I take this

GoZ’s picture

GoZ CreditAttribution: GoZ at Barbe-Rousse commented
Status: Active » Needs review
FileSize
remove_api_doc_groups-2860096-5.patch24.9 KB
jhodgdon’s picture

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

Thanks! The patch looks good -- only removes @defgroup and @addtogroup doc blocks that need to be removed.

Also, I applied it to 4.x and verified that with the patch, all of the things that we want to remove in this issue are removed (there are no more @defgroup, @addtogroup, or @ingroup for update functions groups).

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Version: 8.4.x-dev » 8.3.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed bd69cfc and pushed to 8.4.x. Thanks!

diff --git a/core/modules/datetime_range/datetime_range.post_update.php b/core/modules/datetime_range/datetime_range.post_update.php
index 180a158..b5f3f5d 100644
--- a/core/modules/datetime_range/datetime_range.post_update.php
+++ b/core/modules/datetime_range/datetime_range.post_update.php
@@ -5,7 +5,6 @@
  * Post-update functions for Datetime Range module.
  */
 
-
 /**
  * Clear caches to ensure schema changes are read.
  */
diff --git a/core/modules/editor/editor.post_update.php b/core/modules/editor/editor.post_update.php
index 9bef620..8483900 100644
--- a/core/modules/editor/editor.post_update.php
+++ b/core/modules/editor/editor.post_update.php
@@ -5,7 +5,6 @@
  * Post update functions for Editor.
  */
 
-
 /**
  * Clear the render cache to fix file references added by Editor.
  */
diff --git a/core/modules/hal/hal.install b/core/modules/hal/hal.install
index ea75dcf..78810e3 100644
--- a/core/modules/hal/hal.install
+++ b/core/modules/hal/hal.install
@@ -5,7 +5,6 @@
  * Update functions for the HAL module.
  */
 
-
 /**
  * Move 'link_domain' from 'rest.settings' to 'hal.settings'.
  */
diff --git a/core/modules/migrate/migrate.install b/core/modules/migrate/migrate.install
index 60fb095..064b95a 100644
--- a/core/modules/migrate/migrate.install
+++ b/core/modules/migrate/migrate.install
@@ -5,7 +5,6 @@
  * Contains install and update functions for Migrate.
  */
 
-
 /**
  * Remove load plugin references from existing migrations.
  */
diff --git a/core/modules/path/path.install b/core/modules/path/path.install
index f53c799..fba049e 100644
--- a/core/modules/path/path.install
+++ b/core/modules/path/path.install
@@ -5,7 +5,6 @@
  * Update functions for the path module.
  */
 
-
 /**
  * Change the path field to computed for node and taxonomy_term.
  */

Fixed some coding standards on commit.

  • alexpott committed bd69cfc on 8.4.x 
    Issue #2860096 by GoZ, jhodgdon: Remove api doc groups for updates eg....
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented

Discussed with @xjm and @catch and they +1'd this too. To quote @catch:

It's bad enough getting people to use the right number for updates without this.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Patch (to be ported) » Needs review
FileSize
2860096-8.3.patch21.55 KB

Here's a patch for 8.3.x

jhodgdon’s picture

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

I reviewed this patch the same way I reviewed the 8.4 patch above (see comment #6). Also looks good.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Issue tags: +DX (Developer Experience)

<3

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented

(Saving issue credit.)

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Reviewed & tested by the community » Fixed

As a documentation cleanup, this patch is eligible for commit during the RC phase. Committed 087d90c and pushed to 8.3.x. Thanks!

  • xjm committed 087d90c on 8.3.x 
    Issue #2860096 by alexpott, GoZ, jhodgdon: Remove api doc groups for...

Status: Fixed » Closed (fixed)

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