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

make: add help target to root Makefile for printing info about availble targets #15087

Merged
merged 1 commit into from
Mar 17, 2021

Conversation

fristonio
Copy link
Member

Adds a new help target to the root Makefile that can be used to print information about the available targets from the Makefile. This can be pretty useful given the large number of make targets we have.

Sample:

image

@fristonio fristonio added area/misc Impacts miscellaneous areas of the code not otherwise owned by another area. release-note/misc This PR makes changes that have no direct user impact. labels Feb 23, 2021
@fristonio fristonio requested a review from a team as a code owner February 23, 2021 20:37
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.10.0 Feb 23, 2021
Copy link
Member

@christarazi christarazi left a comment

Choose a reason for hiding this comment

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

🚀 This is great, thanks for the PR! Some nits below

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@aditighag
Copy link
Member

aditighag commented Feb 24, 2021

Super cool! Won't have to reference multiple guides to find a build target now. Could you also add make debug [1] to the help output? Something along these lines -

debug Builds Cilium by disabling inlining and compiler optimizations. Useful for debugging.

[1] https://github.com/cilium/cilium/blob/master/Makefile#L7

@christarazi
Copy link
Member

@aditighag It would also be good to mention that it doesn't strip the binaries of debug symbols as well.

Copy link
Contributor

@twpayne twpayne left a comment

Choose a reason for hiding this comment

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

Very cool!

Makefile Outdated Show resolved Hide resolved
Copy link
Member

@christarazi christarazi left a comment

Choose a reason for hiding this comment

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

LGTM, a non-blocking nit below.

Makefile Outdated Show resolved Hide resolved
Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

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

few nits as below, and maybe cilium -> Cilium.

I like your changes with docker related targets 💯, now i don't need to check Makefile code to find correct target. It's super hard to to do grouping for Makefile.

LGTM 💯

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@sayboras sayboras removed their assignment Feb 24, 2021
Copy link
Member

@tklauser tklauser left a comment

Choose a reason for hiding this comment

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

Nice work, this is really helpful 🚀

Some spelling consistency suggestions inline, mainly s/golang/Go as per https://golang.org/doc/faq#go_or_golang

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
…ble targets

Signed-off-by: Deepesh Pathak <deepshpathak@gmail.com>
Copy link
Member

@tklauser tklauser left a comment

Choose a reason for hiding this comment

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

🚀

@tklauser tklauser removed their assignment Mar 16, 2021
Copy link
Contributor

@twpayne twpayne left a comment

Choose a reason for hiding this comment

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

🍬

@fristonio fristonio added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Mar 17, 2021
@joestringer joestringer merged commit 2186ae4 into master Mar 17, 2021
1.10.0 automation moved this from In progress to Done Mar 17, 2021
@joestringer joestringer deleted the pr/fristonio/make-help-target branch March 17, 2021 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/misc Impacts miscellaneous areas of the code not otherwise owned by another area. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

7 participants