#3361 New Configuration Type Topic

[diazwatson]

Purpose of this pull request

#3361 This pull request (PR) adds a new topic / section to explain how to use the classes described in Overview of module configuration files.

Affected DevDocs pages

• https://devdocs.magento.com/guides/v2.3/config-guide/config/config-files.html
• https://devdocs.magento.com/guides/v2.3/config-guide/config/config-create.html

Links to Magento source code

• https://github.com/magento/magento2/blob/2.3/app/code/Magento/Sales/etc/pdf.xml
• https://github.com/magento/magento2/blob/2.3/lib/internal/Magento/Framework/Config/Data.php
• https://github.com/magento/magento2/blob/2.3/app/code/Magento/Sales/Model/Order/Pdf/Config.php

whatsnew
Added an 'Example of use' section to Create or extend configuration types.

[devops-devdocs]

An admin must run tests on this PR before it can be merged.

[devops-devdocs]

An admin must run tests on this PR before it can be merged.

[diazwatson]

@rogyar
1.

I'm not sure how you were able to create an exact copy of the existing guide :)

Apologies for the mess with branches and PRs, It turned out that my previous PR was extremely out of date, that is how I managed to create an exact copy of the existing guide :)

I believe I have solved the issue by creating everything from scratch.

It would be great if we leave a reference in this topic to the following topic so developers will know where to go for the detailed information.

There is already a reference to the config config-create.html
Do you see a way that can be improved?

Then, the following topic describes the general idea of the configuration file merge.
It would be awesome if we could have a few sentences about how to see (debug) the merged configuration. So, the question is if a developer wants to verify that a custom configuration is present in the merged config, what he is going to do?

I am working on a custom config myself, as soon as I have it working I will be able to provide an example for a developer who wants to verify that a custom configuration is present in the merged config.

[diazwatson]

Hi @rogyar based on my findings while working on a custom config I have recreated the whole page with what I believe is a more mean-full and straight forward explanation of how to extend and create custom configuration files.

Could you please review my changes?

[rogyar]

Hi @diazwatson. Awesome, thank you for the update. And for providing the reference

[diazwatson]

Hey BOT why you assign this PR to me? 🤔

[diazwatson]

@jcalcaben @rogyar guys is there any other way you see this topic can be improved?

[rogyar]

Hi @diazwatson. The only doubtful point that is left is why we have removed the description of all parameters accepted by the ReaderInterface? below. We are still referencing these params here (above)

to provide the following parameters:

But then goes an example and no parameter description anymore.

And thank you for providing the detailed info!

[diazwatson]

Hi @rogyar, thanks for taking the time to review this.

I removed that intentionally because I think the original text is misleading and a bit confusing.

For instance, it used to say

developers should extend an interface
but we all know an interface should be implemented no extended.

Then it said that developers should provide

all these parameters
but in reality if all we need is to create a custom file and we are going to extend \Magento\Framework\Config\Reader\Filesystem (rather than implement the interface) we only need to rewrite the protected attribute $_idAttributes to map the nodes in the xml file.

As a proof, we can see the Reader in the core used as an example:

Model/Order/Pdf/Config/Reader.php

In this reader, only $_idAttributes has been modified to provide a mapping for the xml file nodes.

In this PR I am still mentioning the Interface for those who want to create their own version of the reader.

I have change the paragraphs a bit trying to make this more clear.

Please let me know what you think

[rogyar]

It makes sense. Thank you

[dobooth]

Our bots can be aggressive sometimes...

[rogyar]

From a technical perspective, I'm approving this one. Thank you @diazwatson for providing such extended information.

[diazwatson]

Thank you guys for your help with this @rogyar @dobooth @atwixfirster

[dobooth]

running tests

[dobooth]

A couple link errors. `

• internally linking to /guides/v2.3/guides/v2.3/config-guide/config/config-files.html#config-files-load-merge-merge, which does not exist
  • _site/guides/v2.3/config-guide/config/config-create.html (line 430)
    a href="/guides/v2.3/guides/v2.3/config-guide/config/config-files.html#config-files-load-merge-merge">Configuration file merge
• linking to internal hash #config-files-load-merge-merge that does not exist
  • _site/guides/v2.3/config-guide/config/config-create.html (line 430)
    a href="/guides/v2.3/guides/v2.3/config-guide/config/config-files.html#config-files-load-merge-merge">Configuration file merge`

[diazwatson]

Working on it...

[diazwatson]

Done
@dobooth could you please review?

Is there a way to run these tests locally to detect these issues in advance?

[dobooth]

Sure. In a CLI, at the devdocs root, run rake to build the site. It should return any errors there. You can do rake check:mdl to test the markdown linting on changed files. Try rake -T to show all the checks you can do locally.

[dobooth]

running tests

[ghost]

Hi @diazwatson, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.