-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix rename pages 6303 #6457
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
base: main
Are you sure you want to change the base?
Fix rename pages 6303 #6457
Conversation
✅ Deploy Preview for knative ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Welcome @skmahe1077! It looks like this is your first PR to knative/docs 🎉 |
2d0b95a to
c096fee
Compare
|
cc @dwelsch-esi @iRaindrop for review here |
docs/versioned/.nav.yml
Outdated
| - Install Knative using quickstart: getting-started/quickstart-install.md | ||
| - Knative Functions: | ||
| - About Knative Functions: getting-started/about-knative-functions.md | ||
| - Knative Functions: getting-started/about-knative-functions.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.
I don't want to introduce stutter (eg. #6431)
Should we maybe title these sections Getting Started?
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 will change Knative Functions to "Getting Started"
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.
Modified Getting Started from Knative Functions
| - Developer Tasks: | ||
| - Services: | ||
| - About Knative Services: serving/services/README.md | ||
| - Knative Services: serving/services/README.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.
Don't know the right title here
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’ve removed “About” from the title for consistency with other sections. Happy to update it if there’s a preferred alternative
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 turns out that it doesn't matter in this case, because building the nav causes this README.md page to be organized under Services as a clickable link, and "About Knative Services" isn't shown at all.
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.
oh - that's confusing - what's the best way to represent that in the nav yaml file so it's clear?
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.
One real comment on the top-level title for Knative Eventing (though I could be convinced that we need metadata rather than a longer title), and I agree with Dave that "Getting Started" or moving the files to clickable parent navigation are better for the tutorial concept sections for Functions and Eventing (apparently we never wrote one for serving!)
/approve
(Will wait on the other changes for /lgtm)
| --- | ||
|
|
||
| # Knative Eventing - The Event-driven application platform for Kubernetes | ||
| # Eventing |
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.
Note that because we have explicit .nav.yaml, the titles here aren't reflected in the side nav, so we can have longer titles here and shorter titles in the nav.
My $0.02 here would be to keep the longer name for SEO purposes.
docs/versioned/.nav.yml
Outdated
| - Traffic splitting: getting-started/first-traffic-split.md | ||
| - Knative Eventing: | ||
| - About Knative Eventing: getting-started/getting-started-eventing.md | ||
| - Knative Eventing: getting-started/getting-started-eventing.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.
This should use the same pattern as line 15.
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.
Modified Getting started from Knative Eventing
| - Developer Tasks: | ||
| - Services: | ||
| - About Knative Services: serving/services/README.md | ||
| - Knative Services: serving/services/README.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.
It turns out that it doesn't matter in this case, because building the nav causes this README.md page to be organized under Services as a clickable link, and "About Knative Services" isn't shown at all.
| - Channel based Broker: eventing/brokers/broker-types/channel-based-broker/README.md | ||
| - Apache Kafka: | ||
| - About Apache Kafka Broker: eventing/brokers/broker-types/kafka-broker/README.md | ||
| - Knative Broker for Apache Kafka: eventing/brokers/broker-types/kafka-broker/README.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.
This title change also doesn't matter, because the singleton with README.md gets parented to "Apache Kafka". I do think the longer name in the title makes sense -- it just won't show up in the nav either way.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: evankanderson, skmahe1077 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 |
|
/assign |
c096fee to
b8dbd3d
Compare
Fixes #6303
This PR updates documentation page titles and navigation labels for consistency across the Knative Docs site based on the recommendations in issue.