Update hook_help() for config manager module

this blog explains the UI of this module:
http://drupalize.me/blog/drupal-8-configuration-management-walk-through

• Fixed the path for synchronize config help text
• Added help text for synchronize page
• Added link to documentation for synchronizing config https://drupal.org/node/2182101

Let me know if any omissions/errors.

regards,
Paul

@paul.linney: Thanks for the patch. Is there a reason why we need http://drupal.org/node/2182101 in addition to http://drupal.org/documentation/administer/config? It seems like the help text could just link to http://drupal.org/documentation/administer/config.

Let's also use this issue to review the whole hook_help text of the Config module.

We are reviewing all the hook_help texts for all D8 modules see #1908570: [meta] Update or create hook_help() texts for D8 core modules.

The current text does not refer to the online docs conform the standard help template, see http://drupal.org/node/632280. It should read For more information, see <a href="!translation">the online documentation for the Configuration Manager module</a>.

Another problem is that the about section mentions the the module "provides a user interface". But we do no explain anything about that UI.

There should be a Uses section (again conform the standard help template, see http://drupal.org/node/632280) to explain what the site admin can do with this module, with links to the UI page(s).

Agree with #5 let see if that http://drupal.org/node/2182101 is necessary if we already have http://drupal.org/documentation/administer/config

It seems that Paul's version is more up-to-date. Is that the case?

My page only really covers the synchronization page, so yes it could be included on the original config page or they could be split up into subsections:

• Configuration settings
• Synchronization
• Import Export
• Example Scenarios

If there is agreement on this, then I'll reorganise and fix the help template as suggested and point to the main page (https://drupal.org/documentation/administer/config)

regards,
Paul

We are still trying to make configuration synchronization work: #2121751: [META] Making configuration synchronisation work so it is still rather premature to document how synchronization works. The target use case, as well as a link to the META issue, is described on https://drupal.org/documentation/administer/config.

Another key point that needs to be emphasized wherever configuration synchronization is discussed is that (as far as Drupal core is concerned) synchronization will only work between Drupal installations that are the same site. (For more on this, see #2133325: Create a site UUID on install and only allow config sync between sites with the same UUID.)

Conceptually, I'd argue that there is very little difference between "managing configuration" and "synchronizing configuration" and that we don't need two separate documentation pages. In other words, I think updates should be made to http://drupal.org/documentation/administer/config.

I've updated the https://drupal.org/documentation/administer/config page including the information on the additional post I made and removed the other post.

Doc with all information

regards,
Paul

The other post is this page, right --> https://drupal.org/node/2182101

It has not been removed. You want me to delete it? Or I can move it to ARCHIVE

cheers,
boris

Hi Boris,

Can you delete please, can't see the option to do it myself.

thanks,
Paul

Deleted!

I saved the text as attachment here for posterity

Cheers,

Boris

I added the standard hook_help review task description to the issue summary

Given #9, it seems like maybe we should postpone this issue until the Config module UI is really working?

We just had a change to hook_help, on this issue: #2183113: Update hook_help signature to use route_name instead of path.

Here is the change record: https://drupal.org/node/2250345

This patch will need a reroll for this change.

tagging

Instead of having one critical parent issue I have been asked to change the status of each child issue.

This one doesn't seem to major to me. But yes we should get it done. And it still needs to be postponed because hopefully the UI of this area will be revised.

Actually, I think this issue is at least major. There is pretty much no help at all in config_help() currently, and the concept of synchronization *definitely* needs to be explained to the user, as well as something about config storage.

This is currently postponed, apparently just on #2247291: Reorder tabs in configuration UI.

But that issue is not really active right now, so I think we should un-postpone this issue and at least make sure that the Config Manager module has a hook_help() that documents the current state of the module. If the module is later updated, in theory the patch should include an updated hook_help().

For reference in making this patch, we can reference https://www.drupal.org/documentation/administer/config which is the page/section on drupal.org that explains how to use the config manager module.

Also reducing the scope of the title of this issue, because we have #2376749: [META] Update Configuration System Documentation for the larger issue of making sure config is documented in general.

I'll start on this during DrupalDevDays so that we have something that can be changed as appropriate later.

Thanks Pjonckiere,
this is a good start.

There are some things that should be changed.

a) in the About section: The default wording linking to the documentation that would be For more information, see the online documentation for the Configuration Manager module.

b) also in the About section: We generally refrain from using "Drupal" in the help texts, because the code can also be used for other distributions and users might not be aware that they are using the Drupal core. So better then " between a Drupal installation in different environments", might be something like "between installations of your website in different environments".

c) "deploy a configuration" - that sounds a bit awkward to me. Maybe leaving out the "a" would be better?

We should explain that all configuration is stored in individual yml files. Then the explanation of what the full import/export does would probably be easier.
I think we should change the order: probably first Full export/import, then single and then synchronisation.

For the single export/import: we need to explain where the copy paste comes from, and what's about the dropdown menus and the Entity ID.

Do you have time to work on this, or is it okay if I have a go at it?

@ifrik

I created a patch that fix A) B) C). In addition, I change the order of the "Uses" just like you mention. I did not explain that all configuration are stored in individual yml files. If you provide me the exact text I would created another file explain that.

Here is an screenshot from all results from this patch.

Thanks for the patch! I don't think it's quite ready to commit though:

a) "Development, Staging and Production" --> we use serial commas in Drupal, so this needs to be "Development, Staging, and Production". The same needs to be done elsewhere too.

b) "so you can make and verify your changes with a comfortable distance from your live environment"... what does this mean? I don't think this belongs in the Help. Stick with describing what the module does and how to use it.

c) In the Uses sections... let's not use the word "feature". Just describe what the module does, and if possible make links to the Admin pages where you can do various things.

d) We have a standard for help that says Uses bullet headers should be -ing verbs, so things like "Importing a foo" not "Import".

e) This generally needs some rewriting... maybe read some of the other core hook_help() pages for ideas about the style, and work on the writing style?

If someone provide me the text I can write the patch.

Well if you are at DrupalCon LA maybe you can team up with someone who has writing ideas but not patching skills. Otherwise... for more experienced patchers, it is probably just as easy for them to write the text in the form of a patch. ;)

I'm at DrupalCon LA sitting next to @darol100. I'm going to tackle as much as I can of the items that jhodgdon listed out above starting at a and working down.

Edits for items a-e above (https://www.drupal.org/node/1831798#comment-9912027)

@rhuffstedtler,

I have done a great job by re-writing most of the information on that page. In addition, he have added links to indicate the places where he is talking about.

I have attached an interdif.txt to show the difference between his patch and mine and screenshot of this results.

+++ b/core/modules/config/config.module
@@ -15,15 +15,60 @@ function config_help($route_name, RouteMatchInterface $route_match) {
+      $output .= '<p>' . t('The single export page can be used to retrieve a single configuration in a yml structure.') . '</p>';

I think yml should be YAML. When referring to snippets of code by language, it should be the formal acronym, one shouldn't use the 3 letter file extension to refer to the code snippet.This is especially the case because the current documentation is referring to a yml (YAML) structure.

Unfortunately, this is not particularly consistent in the rest of the file. For instanced, a gzipped tar file might be better off being called as a .tar.gz file. Files should be referred to by file extension. Conversely, code snippets can be called by the formal acronym of the language.

I'm working on this DrupalConLa.

An updated version of this patch with @abenamer suggestion.

Thanks for the work on this patch... But it does not comply with how the other help texts are written or our standards. See http://drupal.org/node/632280 for guidelines, and other modules for examples.

Specific concerns:

1. +++ b/core/modules/config/config.module
   @@ -15,15 +15,60 @@ function config_help($route_name, RouteMatchInterface $route_match) {
   +      $output .= '<p>' . t('For more information, see the online documentation for the <a href="!url">Configuration Manager module</a>', array(
   +          '!url' => 'http://drupal.org/documentation/administer/config',
   +        )) . '</p>';
   +      $output .= '<h3>' . t('Uses') . '</h3>';
   

   For better or for worse, in all other hook_help() implementations, these array() calls within t() are all written on one line. Let's do the same here rather than adopting a different format.

   Also, all drupal.org URLs should be
   https://www.drupal.org/*

2. +++ b/core/modules/config/config.module
   @@ -15,15 +15,60 @@ function config_help($route_name, RouteMatchInterface $route_match) {
   +      $output .= '<dt>' . t('<a href="!url">Exporting a Full Configuration</a>', array(
   +          '!url' => '/admin/config/development/configuration/full/export',
   +        )) . '</dt>';
   +      $output .= '<dd>' . t('Creates a downloadable archive of the active configuration.') . '</dd>';
   

   The DT elements here should NOT be links. That does not follow our standards of other hook_help() implementations. Put the links in the text.

3. +++ b/core/modules/config/config.module
   @@ -15,15 +15,60 @@ function config_help($route_name, RouteMatchInterface $route_match) {
   +      $output .= '<dd>' . t('Allows you to upload a full site configuration from an archive file.') . '</dd>';
   

   The DD sections should describe how to do the thing, and have a link to the page that allows you to do it.

4. +++ b/core/modules/config/config.module
   @@ -15,15 +15,60 @@ function config_help($route_name, RouteMatchInterface $route_match) {
   +      $output .= '<p>' . t('The full export page can be used to export all configurations of this site, and download them as a gzipped tar file.') . '</p>';
   

   "configuration" is not really a countable noun, so we shouldn't be referring to "all configurations" of a site. Probably "the full site configuration" would be better?

   Also we really only want to have these on-page help text chunks if the page is unclear in some way in its UI. This one is probably unnecessary, as are the others? I think the page title and the UI give you the same information. Repeating it in help is not actually desirable.

5. +++ b/core/modules/config/config.module
   @@ -15,15 +15,60 @@ function config_help($route_name, RouteMatchInterface $route_match) {
   +    ¶
   

   There are empty spaces at the end of this line.

update the patch file with @jhodgdon comments.

@rajneeshb, Please do add interdiff.
https://www.drupal.org/documentation/git/interdiff

Second thing when you are uploading a patch based on others suggestion please update the pointers specifically in the comment. like in you updated patch with covered points.

This is looking somewhat better, thanks! I've looked carefully through the latest patch, and here are some suggestions:

a) "...in different environments such as Development, Staging, and Production"
- I don't think these should be capitalized.
- I think we could use a comma after "environments".

b) The second paragraph in About needs a rewrite. There are too many "this" in it to figure out what it means without too much thinking, and it's choppy rather than concise. I think it could be combined with the previous paragraph as well, because it's about the same thing.

c) The link in About to the online docs does not follow our standard format for the link to the online documentation. See the standards on https://www.drupal.org/node/632280 for more details.

d) See the standards... We are using "Sentence case" not "Title Case" for the DT headers in Uses.

e) In the DD elements in Uses... write them in full sentences, not things like "Creates a downloadable archive of the active configuration.". Look at examples or read the standards for style.

f) The link text of links must be accessible, and many of the links here are not. Links need to have descriptive text that will tell you where you go if you click them. Link text like "configuration" is too vague. Links to admin pages should have link text that includes the actual page name in the UI.

g) Please do not split the t() array calls into multiple lines. We are not doing that in other hook_help() functions. See previous review item 1.

h) All of the case xyz: items in the hook_help() need to be indented by 2 spaces, and again, I do not think we need any of these, see previous review item 4.

Re: b, According to the CSM, a comma is only needed if the following phrase is a restrictive clause. Since Development, Staging, and Production don't exhaust the possible set of environments, the comma would be unnecessary. See the fourth example here: http://www.chicagomanualofstyle.org/qanda/data/faq/topics/Commas.html?pa....

I agree that the capitalization is neither necessary nor helpful.

RE #44, it seems like this sentence matches the "needs a comma because it's unrestrictive" example in the page you cited:

Unrestrictive: Don’t you love that lucky, jazzy feeling, such as when you meet someone cute or find money in your pocket? [I love that feeling, unrestricted; here are some examples of it.]

not the Restrictive example:

Restrictive: I love moments such as those. [I don’t love all moments; this tells which moments I do love.]

Here's the sentence in the help:

The Configuration Manager module provides a user interface for importing and exporting configuration changes between installations of your website in different environments such as Development, Staging, and Production.

See, this is providing an example of the "different environments" that you can use Config Manager for, right? It's not saying 'here is the definition of a "different environments" and only this one is valid".

Right? Anyway, I based the "needs a comma" on "it feels like it needs a comma" ;) Turns out I think I agree with the Chicago Manual of Style on this one...

@jhodgdon - you're right. I was tired and misread.

OK, glad we agree. ;) Let's get a new patch based on the suggestions in #43 then?

Here's a new patch. It should address all of the issues. In keeping with the suggestion in #4, after comparing to some of the Uses sections in the other modules, I simplified the text to make it less redundant.

Changing Status

Thanks for the new patch,
I'll review it later today, but already a few points about naming and numbering patches.

The usual naming convention is that the patch contains the issue number and the number of the comment in which it is added. That makes it easier to find in a longer issue queue.

It's also better to keep the rest of the patch name the same, because then it is obvious that a new patch is based on a previous patch. We now got several different patches in this issue, and it's becoming difficult to see whether the later ones are just different contributors saving the continous work with different file names, or whether somebody started from scratch.

And a last point to make reviewing easier: please make an interdiff. That only shows the changes between one patch and another, and that way the reviewers can concentrate on what's changed since the last review.
For details, see https://www.drupal.org/patch

The changes in the last patch took the differences between single and full import/export out, and the links weren't coded according to the routing standards.
Because this issue was now getting a bit messy, I made a new patch directly instead of writing my review up.

Based on the patch in comment #41, I've
- make the link to the online documentation consistent with our standard;
- rewrote all links according to the D8 routing;
- fixed the indent;
- changed the link names to be consistent with the page names (strangely enough there is no page called "Configuration" at all);
- moved the information about UUID from the About section, to the use for which it is relevant;
- added more detail to explain where files are up/downloaded, and where configuration is to be copy-pasted;
- distinguished clearer between YAML format and *.yml files.

I think that should be all.

Thanks for the new patches! This is getting better, but it still needs some work:

a)
Configuration is saved in YAML format in individual <em>*.yml files</em>.
This is not generally true. The default is that configuration is stored in the Drupal database, not in YAML files. So you might want to say that the format for configuration is YAML, but don't say it's saved in individual YAML files. Also I wouldn't use the word "saved", but "stored".

b) Corresponding to that, you'll need to update the wording in some of the other places, because the config can be *exported* to a YML file. For instance, it says "all configuration .yml files of your site" but they're not really normally files in your site, so that would not make sense. It would need to be reworded so that it says you can export it into .yml files.

c) download an archive consisting of all configuration
There is an extra space here after "of".

d) Universally Unique Identifier (UUID) which must match the target site
Needs comma before "which". Also just before this there is an extra space before the word Universally.

e) Actually though

The site is identified using a  Universally Unique Identifier (UUID) which must match the target site to allow the import of configuration files on that site.

This doesn't tell me how to make this UUID or get it into the files, and really there is not a site UUID but each configuration file has one, at least I think so... check that? Anyway, this isn't very useful documentation. It needs to tell me how I can go about being able to export/import configuration, because in general you can't just do it.

f) The configuration and its corrensponding <em>*.yml file name</em> is then displayed on the page for you to copy it
- spelling error: corrensponding
- is => are
- it => them

g) You can to import a single configuration item
Remove word "to"

h) It seems like the Full Export, Full Import, and Synchronizing topics should belong together. Currently they are separated.

i) before setting the changes active.
This needs rewording. You don't "set" changes, really what you're doing is finalizing the import.

j) The Synchronize page also shows new or removed configuration files
- or => and
- Also don't call them files because normally the config active on the site is not stored in files at all.
- And probably it would be better to say something like "configuration items that would be added or removed" (use "would be", because the changes have not been finalized yet).

k) It is important to check that there are no removals of any core configurationfiles
missing space before files, and again don't call them files.

l) such as <em>system.*, entity.*</em>
This is fine language for a programmer, but maybe since this is end-user help, it should say something like:
such as items starting with "system" or "entity"
but really system and entity are not the only things that would cause problems are they? Everything could cause problems. Maybe just leave this whole sentence out?

m)

    case 'config.sync':
-      $output = '';
-      $output .= '<p>' . t('Import configuration that is placed in your staging directory. All changes, deletions, renames, and additions are listed below.') . '</p>';
-      return $output;
+    $output = '';
+    $output .= '<p>' . t('Import configuration that is placed in your staging directory. All changes, deletions, renames, and additions are listed below.') . '</p>';
+    return $output;

The indentation is wrong in all of the case statements. There should still be 2 spaces of indentation.

n)

@@ -36,6 +67,6 @@ function config_file_download($uri) {
   if ($scheme == 'temporary' && $target == 'config.tar.gz') {
     return array(
       'Content-disposition' => 'attachment; filename="config.tar.gz"',
-    );
+      );

This change is wrong.

This is looking very good, thanks! And sorry for the slow response -- I'm pretty behind on reviews.

Anyway, I think we're down to pretty minor things to fix:

• When importing data from a different environment, the site and the import files are matched based on the system.config UUID parameter.

  I think this needs to say that this parameter *must* match, not that the files *are* matched, right? And I dont' think these are "parameters" really... So:
  ... the site and import files must have matching configuration values for UUID in the system.config configuration item.

  Also... how would someone set this up for a site, or where is it generated? I do not know the answer to that, but it might be useful to put in the help, and it might mean the next sentence about generating from a database copy may not make sense.

• Do you think that the Synchronize item should be with the full import/export items? I think it should be, since it is related to the full import, right?
• (full export page help)
  

  The full export page can be used to export all configurations of this site, and download them as a gzipped tar file.

  Should not be "configurations". Should just be "the full configuration" or "all configuration" or "all configuration items". Configuration is not countable, so cannot be pluralized I think.

• (single export page help)
  

  The single export page can be used to retrieve a single configuration in a YAML structure.

  Retrieve? How about sticking with the word "export" here, or maybe "display"?

• The help for the Import page (and the overall module hook_help()) doesn't mention anything about the Staging directory, while the help for the Synchronize page does talk about the Staging directory. So... I think we should either mention the Staging directory in all these places, or in none of them. Probably the Import page should say that it uploads the archive and unzips it, placing the results in the Staging directory, and then you need to go to Synchronize to actually import it into your site, or something like that? I think that is right. The UI is really confusing so I'm not sure. ... #2247291: Reorder tabs in configuration UI is fixing it by the way. We might consider waiting until that is done and then resuming this issue?

That's it.. the rest looks pretty good to me!

Ummm... Maybe comment #59 was meant for a different issue -- I don't see how it's related to this one?

Oh wait, I think you meant we should wait for #2247291: Reorder tabs in configuration UI... Possibly, but that issue may not get in and even if it doesn't, we still need hook_help() to be updated.

Looks great!

Two minor things to fix on the full import page help:

a) "uploading a gzipped tar file" ==> Actually I think this supports other formats as well, like bz... we discussed this recently on #2203779: Improve wording of the configuration import form.

b) "results wll be placed" ==> typo in "will"

And one thing from the single import/export pages (fix in both places):

c) " a single configuration" => " a single configuration item"

Here is a patch base on #63.

@jhodgdon,

a) I'm not sure if what I wrote was right or not.

The interdiff on #64 is wrong. I have attached the correct interdiff in here.

Sorry.

Almost! bz is an alternative to gzip, not an alternative to tar. So I think this should say "...gzipped or bzipped tar...". The other change looks good. Thanks!

Here is a patch base on #66.

Great!

So I took one final look at this patch, from top to bottom. A few typographical/minor suggestions:

a)
"That means that your other environments should initially be set up as a clone ..."

==> should be set up as clones

(the end of this sentence needs to agree in number with the beginning)

b)
"The configuration and its corresponding *.yml file name are then displayed on the page for you to copy them."

I think I would take out the word "them" at the end?

c) Header "Exporting a full configuration" ==> I would replace "a" with "the" here, since it's a definite config you are exporting (the config of your site). But I would leave the "importing" header as it is.

d) help for config.import_full:
"...The full import page can be used to import the full configuration of this site..."

So really you aren't importing the config of *this* site. You're importing config from another site, right? Maybe word this as "... to import a full set of configuration for this site..."? or something similar? Basically "of" => "for" and "the" => "a".

And what do you think about calling (in all the full import/export docs) it a "full set of configuration" instead of "full configuration"? I'm not sure but I am leaning this way...

e) config.import_single:
"...pasting a YAML structure in the textarea."
- "textarea" is not an English word. It's a programmer term. We should say "text box" or "text field" instead.
- probably you paste "into" not "in".

Thanks!

And what do you think about calling (in all the full import/export docs) it a "full set of configuration" instead of "full configuration"? I'm not sure but I am leaning this way...

I do not think we should call in every single place just because you can export your entire website configuration (if I'm not wrong).

-      $output .= '<p>' . t('The full import page can be used to import the full configuration of this site by uploading a gzipped or bzipped tar file consisting of all configuration YAML files. The results will be placed in a the staging directory, so they can be compared in the Synchronize tab. There the import can be finalized.') . '</p>';
+      $output .= '<p>' . t('The full import page can be used to import a full set of configuration for this site by uploading a gzipped or bzipped tar file consisting of all configuration YAML files. The results will be placed in a the staging directory, so they can be compared in the Synchronize tab. There the import can be finalized.') . '</p>';

However, I think in this case apply... What you think ?

This looks excellent. Except:
"should initially be set up as a clones" ===> remove "a" here. Oops.

Great, thank you!

Wow, really excellent work on this one!

Committed and pushed to 8.0.x. Thanks!