-
Notifications
You must be signed in to change notification settings - Fork 392
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
Decouple consts in pkg/daemon #397
Conversation
I'm confused how the osrelease package split helps; it's only used in the daemon already, right? Or is this just for re-organizing and not actually necessary for #396? |
f06cb4c
to
6d9511f
Compare
it's not used just in daemon package, it's also used in cmd/*-daemon package (those are 2 different package) |
oh well, it doesn't help right, it's just better to have it there that way |
6d9511f
to
f2d5a0e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one comment, otherwise looks sane to me! I like the idea of keeping shared consts in a separate pkg to make it easier to tell how things are connected.
f2d5a0e
to
81fb350
Compare
to be used across the whole project. Also, unexport some of the consts that were previously wrongly exported. Signed-off-by: Antonio Murdaca <runcom@linux.com>
81fb350
to
7e80ebd
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jlebon, runcom The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
Just refactor, no change in behavior nor nothing (but do please check!)
TL;DR; consts are the only thing shared and by having their own package we can enable cgo just in daemon (as well as having better code in general, I pretend)
Closes #396 and you can build daemon only with cgo