-
Notifications
You must be signed in to change notification settings - Fork 63
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
Avoid instantiating things via init()
#425
Labels
area/cmd
Issue or PR related to all CLI programs
area/packmanager
Issue or PR is related to KraftKit package managers
kind/maintenance
Issue or PR is general repository maintenance
Comments
antoineco
added
area/cmd
Issue or PR related to all CLI programs
kind/maintenance
Issue or PR is general repository maintenance
area/packmanager
Issue or PR is related to KraftKit package managers
and removed
kind/enhancement
New feature or request
labels
Apr 27, 2023
jake-ciolek
added a commit
to jake-ciolek/kraftkit
that referenced
this issue
Aug 15, 2023
Create a singleton for the umbrella package manager and use it instead of global package-scoped maps. Use more dependency injection. Move the scattered init() calls to RegisterPackageManager into one place. Create a bootstrap package for handling flag/zip registration. Use the bootstrapping package in main(). Propagate errors up and report them if initialization fails. This should make things a bit more testable in the future. GitHub-Fixes: unikraft#425 Signed-off-by: Jakub Ciolek <jakub@ciolek.dev>
4 tasks
jake-ciolek
added a commit
to jake-ciolek/kraftkit
that referenced
this issue
Aug 15, 2023
Create a singleton for the umbrella package manager and use it instead of global package-scoped maps. Use more dependency injection. Move the scattered init() calls to RegisterPackageManager into one place. Create a bootstrap package for handling flag/zip registration. Use the bootstrapping package in main(). Propagate errors up and report them if initialization fails. This should make things a bit more testable in the future. GitHub-Fixes: unikraft#425 Signed-off-by: Jakub Ciolek <jakub@ciolek.dev>
jake-ciolek
added a commit
to jake-ciolek/kraftkit
that referenced
this issue
Aug 16, 2023
Create a singleton for the umbrella package manager and use it instead of global package-scoped maps. Use more dependency injection. Move the scattered init() calls to RegisterPackageManager into one place. Create a bootstrap package for handling flag/zip registration. Use the bootstrapping package in main(). Propagate errors up and report them if initialization fails. This should make things a bit more testable in the future. GitHub-Fixes: unikraft#425 Signed-off-by: Jakub Ciolek <jakub@ciolek.dev>
jake-ciolek
added a commit
to jake-ciolek/kraftkit
that referenced
this issue
Aug 28, 2023
Create a singleton for the umbrella package manager and use it instead of global package-scoped maps. Use more dependency injection. Move the scattered init() calls to RegisterPackageManager into one place. Create a bootstrap package for handling flag/zip registration. Use the bootstrapping package in main(). Propagate errors up and report them if initialization fails. This should make things a bit more testable in the future. GitHub-Fixes: unikraft#425 Signed-off-by: Jakub Ciolek <jakub@ciolek.dev>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
area/cmd
Issue or PR related to all CLI programs
area/packmanager
Issue or PR is related to KraftKit package managers
kind/maintenance
Issue or PR is general repository maintenance
Rationale
Init functions cause imports to have side effects. Side effects are hard to test, reduce readability and increase the complexity of the code.
There are a few places where they prevent us from handling errors properly too:
kraftkit/oci/oci.go
Lines 22 to 25 in 8d4f1a1
Describe alternatives
Instantiate package managers in one place. Don't use globals. Constructors can still live in their own packages.
Register flags centrally and explicitly. If flag definitions need to exist in different packages, simply request them using a public getter.
Interesting read: https://peter.bourgon.org/blog/2017/06/09/theory-of-modern-go.html
Related architectures
None
Related platforms
None
Additional context
No response
The text was updated successfully, but these errors were encountered: