Skip to content

[plugins] Fix init hooks for custom plugins #1500

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 2 commits into from
Sep 22, 2023

Conversation

mikeland73
Copy link
Contributor

Summary

Ensure custom plugin init hooks run. Previously they were only running for builtins.

How was it tested?

devbox run run_test

@mikeland73 mikeland73 requested review from savil and Lagoja September 22, 2023 18:51
echo "MY_INIT_HOOK_VAR environment variable is not set."
exit 1
else
echo "MY_INIT_HOOK_VAR is set to '$MY_INIT_HOOK_VAR'"
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: fix indent. Same for line 7

pluginHooks, err := plugin.InitHooks(devbox.InstallablePackages(), devbox.ProjectDir())
pluginHooks, err := devbox.PluginManager().InitHooks(
devbox.InstallablePackages(),
devbox.Config().Include,
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: should this field be plural: Includes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this ship has sailed, has been in the devbox.json for a while now.

FWIW, I think we chatted about this an decided to use as a verb instead of noun (similar to import instead of imports). I don't feel super strongly, but it seems like it would be difficult to change (but I guess we could support both)

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh yeah, I do remember chatting about it being a verb...

@mikeland73 mikeland73 merged commit e55ef34 into main Sep 22, 2023
@mikeland73 mikeland73 deleted the landau/fix-init-hooks-plugins branch September 22, 2023 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants