Skip to content
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

Add init and pre-init hook defaults as supported config options #542

Merged
merged 5 commits into from Dec 5, 2022

Conversation

mtalexan
Copy link
Contributor

Since the --init-hook and --pre-init-hook are the current method for resolving custom TLS certificates being needed by corporate proxies (see #493), being able to configure fixed hooks that don't need to be manually added on the command-line each time are useful.

The variables already existed and could be overridden by the config files (which are just sourced into the scripts), but the --pre-init-hook was processed strangely and was appending the arguments needed when calling docker at argument parsing-time instead (see #497). The command-line options override the config file options already, so no changes were needed for that.

Unrelated, but there was a typo in the bashate command in the CONTRIBUTING guide as well.

/closes #497

Alexander, Michael added 2 commits November 29, 2022 09:04
Add config variables to example snippet in README.
Allow just the name of the pre-init hook script as the option without the --pre-init-hook prefix.
Fix a typo in the bashate CONTRIBUTING guide.
Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
@89luca89
Copy link
Owner

89luca89 commented Dec 5, 2022

Hi @mtalexan

seems all ok for me, did a couple of changes so we can handle the variable as a normal one, but seems all ok
Will wait for the CI to finish and will merge if all ok 👍

Thanks a lot!

Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
@mtalexan
Copy link
Contributor Author

mtalexan commented Dec 5, 2022

Seems reasonable to me, supporting blank argument for the pre-init-hook in distrobox-init so that it can be blindly passed as part of distrobox-creates call to it.

I don't follow what the GitHub workflow changes are, it looks like there's some outside generation step you ran and then reference as the new config?

@89luca89
Copy link
Owner

89luca89 commented Dec 5, 2022

Seems reasonable to me, supporting blank argument for the pre-init-hook in distrobox-init so that it can be blindly passed as part of distrobox-creates call to it.

Yep 👍

I don't follow what the GitHub workflow changes are, it looks like there's some outside generation step you ran and then reference as the new config?

No it's just GH has deprecated some stuff and the CI was not going ahead so I just fixed this now 🤷

@89luca89 89luca89 merged commit 039c4ad into 89luca89:main Dec 5, 2022
@89luca89
Copy link
Owner

89luca89 commented Dec 5, 2022

Merged, thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Error] --pre-init-hooks doesn't match --init-hooks screwing up .distroboxrc config
2 participants