-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[WIP] 3361 new configuration type #6516
Conversation
An admin must run tests on this PR before it can be merged. |
1 similar comment
An admin must run tests on this PR before it can be merged. |
Hi @rogyar this PR should contain all the changes you recommended me t apply in a previous PR. Could you please have a look and let me know if so see something missing? Thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @diazwatson. I'm not sure how you were able to create an exact copy of the existing guide :)
guides/v2.3/config-guide/config/config-files.md
already exists but this PR shows it as a new file. Maybe there's an issue in the file path (encoding issue) or so.
Also, please, take a look at the review for PR with the same idea. There are some additional ideas that need to be implemented in order to cover all the information requested in the original issue.
Thank you
Thanks, I'll have a look |
@diazwatson , a correct file location - Please pull the latest changes from Thank you! |
Thanks @atwixfirster I got confused because one of the initial recommendation was to add this changes to 2.2 and 2.3 and it was valid for both versions. I'll update this PR with the correct location today. 👍 |
@diazwatson please note, what we do not accept PRs with updates for 2.2 version. So, please add your updates for 2.3 only. Thanks |
Closing in favour of #6514 |
Hi @diazwatson, thank you for your contribution! |
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
Links to Magento source code