Skip to content

Add support for local.yml #242

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
Dec 5, 2022
Merged

Conversation

andrewnicols
Copy link
Contributor

This change adds support for a local yml file to include any customisations that users may wish to include as standard.

This may, for example include changes to existing containers, or it may include the addition of additional containers such as database admin tooling, or proxy servers.

@scara
Copy link
Contributor

scara commented Dec 2, 2022

Hi @andrewnicols,
LGTM!

Is there any reason not to kept aligned even moodle-docker-compose.cmd?

TIA,
Matteo

@andrewnicols
Copy link
Contributor Author

Only because I totally forgot when I wrote this several months ago (and am still amazed that people use Windows for development).

I don't have a Windows machine to be able to test it I'm afraid.

@stronk7
Copy link
Member

stronk7 commented Dec 5, 2022

Only tiny suggestion that I'd do here is to add a comment about ensuring that the loading of the local.yml always remains the last one before calling compose. Just to avoid forgetting about that in the future.

Other than that, I think this is the correct way to go, enabling virtually any customisation without overburdening configuration. And, at the same time, it also makes it really easier to test thing that may end into the main scripts if good enough. So big +1 here.

I'll run some basic test under Unix and Windows soon...

@stronk7
Copy link
Member

stronk7 commented Dec 5, 2022

Ah, and would be great if we can add the very same ECHO "Including local options from... to the Windows one. Sorry just saw it now.

This change adds support for a local yml file to include any
customisations that users may wish to include as standard.

This may, for example include changes to existing containers, or it may
include the addition of additional containers such as database admin
tooling, or proxy servers.
@andrewnicols
Copy link
Contributor Author

Only tiny suggestion that I'd do here is to add a comment about ensuring that the loading of the local.yml always remains the last one before calling compose. Just to avoid forgetting about that in the future.

Done :)

Ah, and would be great if we can add the very same ECHO "Including local options from... to the Windows one. Sorry just saw it now.

And done - hope I got this right.

@stronk7
Copy link
Member

stronk7 commented Dec 5, 2022

Thanks, looks perfect! Will test shortly...

@stronk7
Copy link
Member

stronk7 commented Dec 5, 2022

Tested both under Unix and Windows, it worked great, so far!

@stronk7 stronk7 merged commit 8256be5 into moodlehq:master Dec 5, 2022
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