-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Conversation
Add Docfile that contatins configuration for devdocs content management Implement rake automation to use the Docfile locally and on CICD
Why is the configuration file not using something standard like YAML or JSON? |
The natural language format is more user-friendly for writers. No need to delve into implementation to configure the website and no obsolete documentation in comments (comparing with |
Add a blank line at the end of file
This is a really interesting and clever proposal @dshevtsov! It certainly puts a "writerly" spin on configuration, haha! However, I have reservations about creating an unconventional configuration pattern when all other site configuration uses YAML. It creates another layer of complexity that requires unique handling in our automation scripting, which using a more standard format like YAML presumably does not. It may also insult some writers who may misinterpret the solution as a lack of confidence in their ability to work with a more conventional format. I'm not saying that it's your intention, but it could be interpreted that way. Perhaps if you articulated the benefits of this format over a more conventional format, it would be easier to have a constructive conversation about the pros and cons. |
What if we generate this verbose explanation of the readme file? So configuration will be in YAML, but the output of the config will be explained in the plain english? |
I'm still not sure what problem this new format addresses. JSON and YAML are already declarative language formats with robust parsing libraries that are available in multiple programming languages. This approach limits reusability and forces us to re-implement the parsing logic if we ever decide to move away from If readability is the issue, I argue that the following is just as easy to read and understand in the context of a configuration file: {
"repositoryDependencies": [
{
"githubUrl": "magento/magento2-functional-testing-framework",
"branch": "develop",
"sourceDirectory": "docs",
"targetDirectory": "mftf",
},
{
"githubUrl": "magento/devdocs-m1",
"branch": "master",
"targetDirectory": "m1x",
}
]
} |
I agree with James, but I prefer YAML over JSON. JSON is more error prone when it comes to manual editing in my opinion. |
It doesn't address some specific problem. It just shows another way of a configuration file use. The file is very simple, so why do we use YAML and JSON when we can use English. No need to use other libraries, only regular expressions functionality that is very probably be the same in different languages. |
I worry that the natural language syntax may become unwieldily over time if we need to extend the |
The format can be changed when the configuration will become complex enough. I'm changing the format to YAML, will update it in this PR. |
…into ds_MAGEDOC-3692_docfile
Changed Docfile to Docfile.yml. |
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.
Thanks @dshevtsov. I tested this branch locally and it works well. It's definitely an improvement over hardcoding all the shell script options in the rake init
task.
Before approving this PR, I'd like to try something different since this impacts our Jenkins pipelines as well. We need to start creating changes to pipeline scripts in git. I know that can be inconvenient since we often need to test changes directly in the Jenkins console before we know precisely which changes we need to make or which changes will work. However, I think it's something we need to start getting used to.
Please create a PR for all impacted Jenkins pipelines and link to this PR.
It doesn't look like our new multirepo-build-deploy-staging
pipeline has been added to the infrastructure repo yet. I'll add it so you can create a PR.
running tests |
running tests |
Hi @dshevtsov, thank you for your contribution! |
Purpose of this PR
This PR:
rake init
task to use the Docfile on CICD and locallyAdditional information
Internal ticket: MAGEDOC-3692