Skip to content

[FEATURE] Search for configuration in parent of vendor #571

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 8, 2023

Conversation

linawolf
Copy link
Contributor

@linawolf linawolf commented Sep 2, 2023

If the script is executed from any other locatation but its own project directory we need to search for the guides.xml above the vendor.

Copy link
Contributor

@wouterj wouterj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In #482, I've introduced a global --config option (but forgot to mention it). I'm not 100% sure we should make this change, looking from the install location for config files doesn't make much sense to me.

With the --config option, the tool will either find the guides.xml in the current directory (default) or a user has to specify the path to the guides.xml file using e.g. --config=.Build

@linawolf
Copy link
Contributor Author

linawolf commented Sep 5, 2023

@wouterj for us it makes a lot of sense. We have a docker container that is prepared once and needs certain configurations like for the extensions that it uses etc. This docker container can then be executed in any directory that contains a docs or Documentation folder. These users do not know about the internal workings of the guides and do not need to know about them. So we need the configuration where we have the installation not where we have the content. Also as we make the parameters configurable just setting a --config param also would not really work as this would mean that ppl using the docker container about the location of the config file within that container. Also I really like @jaapio s idea about applying multiple configurations. We could then use the configuration file in the installations path to define the Extensions to be used, settings for the theme etc and use an additional guides.xml in another location to additionally define stuff like intersphinx linking or project titles or whatever

@jaapio
Copy link
Member

jaapio commented Sep 6, 2023

@linawolf what if we would change your docker container setup a bit... Right now you are using

ENTRYPOINT ['???/.Build/bin/guides'] if you change this to ENTRYPOINT ['???/.Build/bin/guides', '--config=???/.Build/'] you would always load the config from the build directory.

The downside of this is that users of the container cannot overwrite it... but I'm not sure that would ever be an use case for you?

@linawolf
Copy link
Contributor Author

linawolf commented Sep 7, 2023

@jaapio with the recent changes of wouter it might make sense to have the configuration file on both project and application level. If we start using it for configurations like intersphinx, projectname etc?

@jaapio
Copy link
Member

jaapio commented Sep 7, 2023

We can do that, but it requires you to allow passing multiple levels, as you want to put it some base config in your docker container for interproject config?

If the script is executed from any other locatation but its own project directory we need to search for the guides.xml above the vendor for the general application settings. These can be combined with the project specific settings from the parameter of the command
@linawolf linawolf changed the title [FEATURE] Search for configuration in parent and grandparent of vendor [FEATURE] Search for configuration in parent of vendor Sep 8, 2023
@linawolf
Copy link
Contributor Author

linawolf commented Sep 8, 2023

@jaapio I combinded the configurations and dropped the grandparent of vendor to not make it to complicated. I can also copy the guides.xml into the .Build folder or just ommit the .Build folder all together in my docker container

@jaapio jaapio merged commit 425754d into main Sep 8, 2023
@jaapio jaapio deleted the feature/config branch September 8, 2023 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants