-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
remove remap-rootfs bin when running make clean #4139
remove remap-rootfs bin when running make clean #4139
Conversation
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.
LGTM, though I wonder if it would be better to have a per-contrib-cmd Makefile (and move the .gitignore
entries while we're at it). Then again, we configure the go build
line fairly heavily so maybe that wouldn't work too well...
.PHONY: clean | ||
clean: |
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.
Was there a reason you moved this to a different location in the Make file? (it makes it harder to see what changed)
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.
I think the idea is to make it harder to miss when we add new targets in the future:
So I think move the target clean next to all will reduce such type error in the future?
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.
Ah, yeah, probably; was considering 2 commits (one that moves it, and one that updates it), but not a hard blocker.
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.
Thanks!
Another think, as @kolyshkin asked in #4140 (comment) , I think we should make one more 1.1.x release because of the issue #4122 . But it's very hard to backport #4124 to release-1.1
, because in this PR, there are two topics in one commit, while one topic of them is not in release-1.1
. @cyphar
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.
That backport is ready in #4144.
253c2f4
to
960bc7e
Compare
Signed-off-by: lfbzhm <[email protected]>
Signed-off-by: lfbzhm <[email protected]>
f16fea3
to
c811308
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.
LGTM, though I think splitting the makefile might make more sense (we can do that in a future PR).
I have reviewed and found such errors for about 2 times.
So I think move the target
clean
next toall
will reduce such type error in the future?