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.

Sign up for GitHub

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

Jump to bottom

daemon: Add hidden --cflags debug command #15549

Merged
merged 1 commit into from
Apr 6, 2021
Merged

daemon: Add hidden --cflags debug command #15549

merged 1 commit into from
Apr 6, 2021

Conversation

joestringer
Copy link
Member

Support compiling with dedicated compile options to assist development.

@joestringer
daemon: Add hidden --cflags debug command
9979891 
Support compiling with dedicated compile options to assist development.

Signed-off-by: Joe Stringer <joe@cilium.io>
@joestringer joestringer requested a review from a team April 2, 2021 21:48
@joestringer joestringer requested review from a team as code owners April 2, 2021 21:48
@joestringer joestringer added the release-note/misc This PR makes changes that have no direct user impact. label Apr 2, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. and removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Apr 2, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.10.0 Apr 2, 2021
borkmann
borkmann approved these changes Apr 6, 2021
View reviewed changes
Copy link
Member

@borkmann borkmann left a comment

Choose a reason for hiding this comment

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

Do you have a specific example that you typically use?

@joestringer
Copy link
Member Author

@borkmann there are a few other flags we could maybe make better use of like -Wundef, but in general just having this capability at all is useful to play around with newer clang flags in the container we deploy rather than locally based on my own environment setup.

@joestringer
Copy link
Member Author

Given that the flag is hidden, I don't think there's a big deal to wait for the remaining @cilium/cli codeowner review (especially given there's already four reviews). Good to merge.

@joestringer joestringer merged commit ca31483 into cilium:master Apr 6, 2021
1.10.0 automation moved this from In progress to Done Apr 6, 2021
@joestringer joestringer deleted the submit/better-compiler-debugging branch April 6, 2021 21:54
This was referenced Apr 28, 2021
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@borkmann borkmann borkmann approved these changes

@christarazi christarazi christarazi approved these changes

@qmonnet qmonnet qmonnet approved these changes

@vadorovsky vadorovsky vadorovsky approved these changes

@errordeveloper errordeveloper Awaiting requested review from errordeveloper

Assignees

@errordeveloper errordeveloper

Labels
release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.10.0
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@joestringer @borkmann @christarazi @qmonnet @vadorovsky @errordeveloper