-
Notifications
You must be signed in to change notification settings - Fork 248
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
Docs: Fix the link to feature gates documentation #1821
Conversation
Welcome @ChaoyiHuang! |
Hi @ChaoyiHuang. 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. |
✅ Deploy Preview for kubernetes-sigs-nfd ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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 you @ChaoyiHuang for the contribution. Please sign the CLA so we can get this merged 😊
/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.
Now that we're at it, could you fix the same thing in docs/reference/master-commandline-reference.md
, too?
@@ -34,7 +34,7 @@ Print version and exit. | |||
|
|||
The `-feature-gates` flag is used to enable or disable non GA features. | |||
The list of available feature gates can be found in the [feature gates | |||
documentation](../feature-gates.md). | |||
documentation](feature-gates.md). |
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.
Looking at the netlify preview it didn't work (perhaps some bug in some 3rd party component in our docs building pipeline) 🤔 Maybe the links should not be split accross multiple lines(?)
documentation](feature-gates.md). | |
[feature gates documentation](feature-gates.md). |
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.
Looking at the netlify preview it didn't work (perhaps some bug in some 3rd party component in our docs building pipeline) 🤔 Maybe the links should not be split accross multiple lines(?)
will check to see how to address it
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.
I just found why it doesn't work in netlify preview.
In netlify preview, if you click the item in the navigation bar "10.Feature Gates", it's pointing to "https://deploy-preview-1821--kubernetes-sigs-nfd.netlify.app/reference/feature-gates", and it works there. There is no ".md" for the page URL.
But in the "master-commandline-reference" page, the link to Feature Gates including suffix ".md" in the URL, that means URL "https://deploy-preview-1821--kubernetes-sigs-nfd.netlify.app/reference/feature-gates.md" won't work.
The jekyll should convert the relative link (feature-gates.md) to (feature-gates.html), but it didn't.
Need to figure out how to address the issue.
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.
It should change the .md -> .html automatically when building the site. Do what I suggested above^ and I think it fixes it
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.
yes, it's the multi-line issue, and some bug in jekyll rendering.
Both .md link and html link work Now.
Yes, I am waiting for CLA approve from corporate CLA Manager. |
2875461
to
3e75769
Compare
The link to feature gates documentation is pointing to the upward folder in master-commandline-reference.md, it should be updated to linking file in the same folder. Signed-off-by: joehuang <[email protected]>
Closes #1823 |
Thanks @ChaoyiHuang. The worker cmdline reference is also broken similarly, could you fix that, too? |
Sure, I will check other links and fix if necessary |
The link to feature gates documentation is pointing to the feature-gates.md in master-commandline-reference.html and worker-commandline-reference.html, it should be updated to linking html file. Signed-off-by: joehuang <[email protected]>
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.
Thank your @ChaoyiHuang for fixing the links
/lgtm
docs/Gemfile
Outdated
# render realtive link by plugin | ||
# https://github.blog/news-insights/product-news/relative-links-for-github-pages/ | ||
gem 'jekyll-relative-links' |
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.
Don't seem to help
# render realtive link by plugin | |
# https://github.blog/news-insights/product-news/relative-links-for-github-pages/ | |
gem 'jekyll-relative-links' |
LGTM label has been added. Git tree hash: 643dd0ab1c6b3412736c5b111fd984838671cecb
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ChaoyiHuang, marquiz 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 |
The link to feature gates documentation is pointing to the
feature-gates.md in master-commandline-reference.html, it should be updated to link html file.