-
Notifications
You must be signed in to change notification settings - Fork 363
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
Add prototype documentation site #514
Conversation
@ahmetb: The label(s) In response to this:
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. |
- developed with Hugo, though the design is currently quite minimal as I am not focusing on those parts right now - imported User Guide (install docs still will live in the repo README) - imported Developer Guide and made some heavy editing, added new pages I recommend we merge this as-is and continue iterating on it over time. To try this patch out: cd site; hugo serve Signed-off-by: Ahmet Alp Balkan <ahmetb@google.com>
Codecov Report
@@ Coverage Diff @@
## master #514 +/- ##
=========================================
+ Coverage 58.51% 59.12% +0.6%
=========================================
Files 22 23 +1
Lines 969 1003 +34
=========================================
+ Hits 567 593 +26
- Misses 346 350 +4
- Partials 56 60 +4
Continue to review full report at Codecov.
|
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 great work! 🎉 🍾
I left a few comments. Let's do only this single round of review and then let's iterate on new PRs as suggested.
Some related points:
-
When serving locally with
hugo serve
, code blocks did not have a dark background. Can you check how it renders on your machine? -
For easier navigation it would be nice to have links to the next/previous page of the documentation. Is that doable?
files: | ||
- from: "kubectl-restart-*/restart.sh" | ||
to: "./restart.sh" | ||
- from: "LICENSE" |
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 doesn't say anywhere that we require a license to be included and installed. Let's add something like
# make sure to always install your LICENSE file
EDIT Ah, I found that you added a not to the submitters checklist. So this comment is not super urgent, but maybe still helpful.
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.
Wontfix for now.
Yeah it’s expected since it’s not in this PR. :) |
Code review suggestions. Co-Authored-By: Cornelius Weig <22861411+corneliusweig@users.noreply.github.com>
Signed-off-by: Ahmet Alp Balkan <ahmetb@google.com>
Did another pass, PTAL @corneliusweig. |
This is pure gold! Thanks again for getting this done! |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahmetb, corneliusweig 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 |
as I am not focusing on those parts right now. Therefore, Hugo-specific
parts are <40 lines.
I recommend we merge this as-is and continue iterating on it over time.
To try this patch out:
cd site; hugo serve
/kind docs
Related issue: #509.