Problem/Motivation

@webchick pointed out in that all the constants introduced for #2120003: [META] Create sensible limits for the maximum length of configuration object filename components are a little confusing, and the docs could be made more explicit to clarify what the different parts of the filename are and why the various values were chosen.

Proposed resolution

Improve the docs for all the relevant contstants with more detailed explanation and @see to other relevant constants.

   |\      _,,,---,,_
   /,`.-'`'    -.  ;-;;,_
  |,4-  ) )-,_..;\ (  `'-'
 '---''(_/--'  `-'\_) 
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

xjm’s picture

Status: Active » Needs review
FileSize
3.46 KB
xjm’s picture

+++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityType.php
@@ -19,13 +19,31 @@ class ConfigEntityType extends EntityType {
+   * 1. The config prefix, which is returned by getConfigPrefix and is

Oops, should be getConfigPrefix().

Not rerolling until someone else reviews. ;)

xjm’s picture

+++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityType.php
@@ -19,13 +19,31 @@ class ConfigEntityType extends EntityType {
+   * config prefixes if desired.

...And this should be "namespace identifiers if desired" to match the words we used earlier in the docblock.

jhodgdon’s picture

The explanations seem understandable to me. A few formatting nitpicks:

a) This is not going to format well on api.drupal.org:

+   * Configuration entity names are composed of two parts:
+   * 1. The config prefix, which is returned by getConfigPrefix and is
+   *    composed of:
+   *    a. The provider module name (limited to 50 characters by
+           DRUPAL_EXTENSION_NAME_MAX_LENGTH).
+   *    b. The module-specific namespace identifier, which defaults to the
+   *       configuration entity type ID. Entity type IDs are limited to 32
+   *       characters.
+   * 2. The configuration entity ID.

It doesn't support number/letter lists, only bullets with - ... so this will format as all one jumbled-up paragraph. Not too great...

b) Also getConfigPrefix should probably end in () -- I assume it's a method on this class?

c) And one of those lines in there is missing a * at the start.

d)

+   * @see \DRUPAL_EXTENSION_NAME_MAX_LENGTH

Why the \ here? If this is just a constant defined in an include file, I think we should just leave off the \ ?

jhodgdon’s picture

Status: Needs review » Needs work
jhodgdon’s picture

Also, is it bytes or characters?

xjm’s picture

Thanks! It's characters. I'll reroll to fix the formatting once someone else who's worked on these issues can review it.

Berdir’s picture

Documentation looks correct to me, can RTBC when the wording/formatting stuff is fixed.

xjm’s picture

Status: Needs work » Needs review
FileSize
3.56 KB
2.22 KB

Here we go. Also added an @see to the constant from #1709960: declare a maximum length for entity and bundle machine names.

xjm’s picture

One additional clarification.

Berdir’s picture

Status: Needs review » Reviewed & tested by the community

I think all feedback has been addressed, looks correct, so RTBC.

xjm’s picture

Two more teensy fixes (a space and a typo, so leaving RTBC).

xjm’s picture

Sigh, and more whitespace. Clearly have lost my touch.

  • Commit f8c8106 on 8.x by jhodgdon:
    Issue #2229931 by xjm, Berdir: Improve documentation related to config...
jhodgdon’s picture

Status: Reviewed & tested by the community » Fixed

Latest patch looks good. How about if I just commit it? OK. :) Done!

Status: Fixed » Closed (fixed)

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