Skip to content
This repository was archived by the owner on Mar 18, 2022. It is now read-only.

Tests for traversal merging #31

Open
agjohnson opened this issue Nov 15, 2017 · 4 comments
Open

Tests for traversal merging #31

agjohnson opened this issue Nov 15, 2017 · 4 comments
Labels
Needed: more information A reply from issue author is required Needed: tests Tests are required

Comments

@agjohnson
Copy link
Contributor

Looking quickly, it doesn't appear we have tests to test how a configuration file is merged if a nested configuration file is found.

I would expect:

  • Both files are found
  • The top level readthedocs.yml configuration has precedence
  • The nested readthedocs.yml doesn't merge in its keys to our configuration
@agjohnson agjohnson added Needed: more information A reply from issue author is required Needed: tests Tests are required labels Nov 15, 2017
@stsewd
Copy link
Member

stsewd commented Feb 25, 2018

I don't fully understand this, at the end the top level configuration file is the one that is considered to load the settings and the nested file is ignored?

@agjohnson
Copy link
Contributor Author

There were some wonky ideas around the spec for our config early on. I do believe that child readthedocs.yml files will merge into the parent file. I'd probably say we don't want this behavior at all, right? If i have a parent readthedocs.yml file, there should be no case where I want my child readthedocs.yml file parsed. The only case I can think of is a repo with multiple projects in it, and potentially multiple readthedocs.yml files.

@agjohnson agjohnson added this to the Test improvements milestone Mar 6, 2018
@ericholscher
Copy link
Member

+1 to removing this, and only supporting one file. I believe the "rtd.yml selection" code already does this, so we should remove the complexity in parsing that is needed for this support.

@stsewd
Copy link
Member

stsewd commented Mar 8, 2018

I search in all the code and couldn't find nothing related to merging configuration files

There is only one test that has nested configuration files

https://github.com/rtfd/readthedocs-build/blob/15ad6112b3e87e72c22182af3b1765d35c19ea82/readthedocs_build/config/test_config.py#L95-L103

But here the filter isn't needed, since the code only loads and parse one configuration file (about this should be the test, then? Test that the code only parse one configuration file on load?).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Needed: more information A reply from issue author is required Needed: tests Tests are required
Projects
None yet
Development

No branches or pull requests

3 participants