-
Notifications
You must be signed in to change notification settings - Fork 498
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
Improve feature name readability in conformance reports #3564
base: main
Are you sure you want to change the base?
Conversation
Hi @08volt. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
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 for this PR, @08volt!
I appreciate the effort of making our implementation table better, but I have a couple of comments after having looked at the preview https://deploy-preview-3564--kubernetes-sigs-gateway-api.netlify.app/implementations/v1.2:
- The prefixes
Gateway
andHTTPRoute
still make sense to be included in the feature name, as it's useful to understand whether the feature is related to the Gateway or the Route object. - Capitalized words (e.g.,
HTTP
) now include a weird spacing (HTTP
->H T T P
)
Add feature name processing to remove redundant prefixes and improve readability in conformance comparison tables. Changes include: - Remove "HTTPRoute" and "Gateway" prefixes from feature names - Split camelCase words into space-separated words - Add process_feature_name() function for consistent text processing - Update generate_profiles_report() to use processed feature names This makes the conformance reports easier to read
ce5c24b
to
106299e
Compare
Nice catch, I improved the regex to keep the capital consecutive letters together. |
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 for fixing this. Some words still have a weird spacing though, such as H2C
-> H 2 C
. Could you also put back the Gateway
and HTTPRoute
prefixes, please?
/cc @xtineskim
as you might be interested in this change.
/ok-to-test |
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! thanks for adding this in 😁
if you mkdcos serve in this pr, you can apply the changes to the tables 👍
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 08volt, xtineskim The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/kind documentation
What this PR does / why we need it:
Add feature name processing to remove redundant prefixes and improve readability in conformance comparison tables. Changes include:
This makes the conformance reports easier to read
Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?: