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

hive: Add title to Module() and enforce format #21915

Merged
merged 1 commit into from
Nov 1, 2022

Conversation

joamaki
Copy link
Contributor

@joamaki joamaki commented Oct 28, 2022

To support short module identifiers for use in logs the module name is now an identifier with forced format (lower-case, 30 chars).

This however hampers readability when visualizing the hive, so for this purpose we add a title to the module. This is also forced to only contain alpha-numeric characters and be at most 80 characters in length to mostly keep the module line in "PrintObjects" short enough to fit on one line.

@joamaki joamaki requested review from a team as code owners October 28, 2022 14:46
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 28, 2022
To support short module identifiers for use in logs the module
name is now an identifier with forced format (lower-case, 30 chars).

This however hampers readability when visualizing the hive, so for
this purpose we add a title to the module. This is also forced
to only contain alpha-numeric characters and be at most 80 characters
in length to mostly keep the module line in "PrintObjects" short enough to
fit on one line.

Signed-off-by: Jussi Maki <jussi@isovalent.com>
@joamaki joamaki force-pushed the pr/joamaki/hive-module-titles branch from 76d0cc9 to c4f3b5f Compare October 28, 2022 14:55
@joamaki joamaki added the release-note/misc This PR makes changes that have no direct user impact. label Oct 28, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 28, 2022
Copy link
Member

@dylandreimerink dylandreimerink left a comment

Choose a reason for hiding this comment

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

LGTM!

@joamaki
Copy link
Contributor Author

joamaki commented Oct 31, 2022

Marking ready-to-merge as we've two relevant reviews.

@joamaki joamaki added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Oct 31, 2022
@aanm aanm merged commit b8e8ca4 into cilium:master Nov 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants