-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Keep project name display aligned #9920
Conversation
This issue is currently awaiting triage. If Ingress contributors determines this is a relevant issue, they will accept it by applying the The 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/test-infra repository. |
Welcome @guessi! |
Hi @guessi. Thanks for your PR. I'm waiting for a kubernetes 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/test-infra repository. |
@guessi thanks for the contribution
|
@@ -1,4 +1,4 @@ | |||
site_name: NGINX Ingress Controller | |||
site_name: Ingress-Nginx Controller |
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.
This is the only non-md update in this PR.
I believe it should be the title of the Project Homepage, should be aligned as well.
@longwuyuan I've reverted the changes to |
@guessi please ceck my comment earlier about the README inside the carts directory |
@longwuyuan I've updated the |
@guessi can you leave the /charts directory out of the PR for now. We can add it after the tests pass. Te flaking tests are a known issue so please remember to keep rebasing if main head gets ahead. |
Sure, no problem, will update in minutes. |
later when you do want to change the README under /charts, please read the link I posted earlier about helm-docs tool. It means you have to edit the values yaml and use the helm docs tools to generate the README file. That file is not to be edited manually |
Just updated, I removed the changes to Actually, I do follow the instructions to update $ vim charts/ingress-nginx/values.yaml
$ vim charts/ingress-nginx/README.md.gotmpl # <---- Is this the issue?
$ helm-docs ...
$ git diff --exit-code
$ git add charts/*
$ git commit ... but I have no idea why it still failed. |
For what I see, most recent PRs are all now failing at the same check items, I believe there might have something went wrong before the PR created. Anyway... I've removed all the changes to |
Thanks @guessi |
/triage-accepted |
/lgtm Thank you for this. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: guessi, longwuyuan, strongjz The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Thanks for merging this. One quick question, should I create a new PR for Let me know what I could helps. |
@guessi try new PR for /charts/* . You would need to edit values.yaml and then use helm-docs. Ask questions in the dev channel if tests fail |
What this PR does / why we need it:
To keep project name display aligned across all documentations.
** To keep minimal risk of change, the PR will focus on
*.md
files only.Types of changes
Which issue/s this PR fixes
Please refer to #9918 for more details.
How Has This Been Tested?
n/a
Checklist:
Does my pull request need a release note?
No