-
Notifications
You must be signed in to change notification settings - Fork 901
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 hugo migration #3337
Docs hugo migration #3337
Conversation
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 awesome effort @plumbis! Left a few comments / questions below :)
@@ -51,6 +47,3 @@ in an `override.yaml` file would look like this: | |||
name: ca-bundle-config | |||
key: ca-bundle | |||
``` | |||
|
|||
|
|||
[Install Crossplane]: ../reference/install.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.
Is this link unique in some way that makes it better to make inline above?
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, when using {{<ref ... >}}
shortcode Hugo will find the markdown page that is associated with this path/title and put it in the markdown. The result should be identical to the markdown engine.
The magic is that if the page moves or is renamed, the link breaks. With traditional markdown links this is just a silently broken link. By using the shortcode Hugo will fail to start. This draws attention to update the link.
99% of the time we're not trying to help the person writing the link but trying to notify the person moving pages to update the links pointing to that page.
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.
Gotcha, thanks for the explanation! I think this is a reasonable outlook and ultimately will lead to us having higher quality documentation. We are paying a cost in review as folks cannot see the links in the GitHub rendered markdown, but that feels like the right trade-off to make (i.e. let's rely on Hugo machinery to not let us publish instead of human eyes noticing bad links). That being said, I don't think we need the whole Hugo pipeline in this repo necessarily, but we should run at least enough to verify that links are not broken in CI so we can avoid merging bad content.
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.
Right now the only thing we have is the content, none of the layout or "shortcodes" that render links like that. Those live exclusively in the Crossplane.io repo (example) .
Historically CI (for the links) has been done by just running hugo against the complete repo- if it compiles we pass, if it doesn't then there's something wrong (e.g., broken link). To do that we would have to bring in the rest of the hugo files and configurations.
Defining this process doesn't need to be a blocker for this PR but I think we can checkout the main/master branch of crossplane.io and copy the docs from this repo into the clone and run hugo. I don't imagine this will be very complicated.
@@ -0,0 +1,187 @@ | |||
--- | |||
title: "Crossplane Documentation" |
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.
We typically try to wrap docs at 80 characters to make it easier to comment on smaller sections. Would you be open to doing that in this doc (and potentially add it as part of this guide as well)?
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 think this works well for text and makes it harder to read in an editor.
Not only is it unlikely to be viewed on a terminal where the line limit makes sense but it becomes very messy when you're including links. For example the link alone is 70 characters.
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 think this works well for text and makes it harder to read in an editor.
We have had pretty strong consensus on doing this in the past due to the GitHub suggestion granularity, but I am inclined to defer to the person who literally writes documentation for a living :) If there is strong push back we can always change in the future, and we can preserve the practice of wrapping in design docs for now where we are perhaps going to give more granular feedback.
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.
+1 to being willing to change in the future and thanks for being flexible.
@@ -1,7 +1,5 @@ | |||
--- | |||
title: provider-alibaba |
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.
Just double checking, these provider docs will show in TOC right?
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, that's why this looks like a weird implementation. The pages have to be here with this metadata to appear in the menu but we can't enforce a redirect from the markdown page so there are a few hoops that have to be jumped through to make it happen.
You can see in the preview site for the crossplane.github.io PR. https://master.d1bmjmu5ixie8d.amplifyapp.com/v1.9/api-docs/
@@ -49,10 +46,10 @@ learn more about all of these concepts in the [composition documentation]. | |||
|
|||
<!-- Named Links --> | |||
|
|||
[Packages]: packages.md | |||
[Packages]: {{<ref "packages" >}} |
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.
Is the ref
syntax required? Not a huge deal but is kind of a bummer that they don't render as markdown links on GitHub.
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.
No, it's not, but to the earlier comment, it's an easy and fast way to detect broken links.
Although it doesn't render on github I think you'll get more value by running hugo locally and using that as a method for review, since that provides the visuals as well.
|
||
|
||
### Directory structure | ||
The relevant directories of the docs repository are: |
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 directory structure for the docs site repository right? It might be worth mentioning that we are not talking about crossplane/crossplane
.
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 updated this to be a link to crossplane/crossplane.github.io
repo to clarify.
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 @plumbis! We are going to follow this up with some pre-merge checks that ensure content builds correctly 👍🏻
d949690
to
a083ced
Compare
Signed-off-by: Pete Lumbis <pete@upbound.io>
Signed-off-by: Pete Lumbis <pete@upbound.io>
Signed-off-by: Pete Lumbis <pete@upbound.io>
Add a link to the website repo to clarify that the reference to the `docs repository` is not talking about crossplane/crossplane but is talking about the docs in the context of the site. Signed-off-by: Pete Lumbis <pete@upbound.io>
Signed-off-by: Pete Lumbis <pete@upbound.io>
Signed-off-by: Pete Lumbis <pete@upbound.io>
a083ced
to
734d64d
Compare
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 all of your effort here @plumbis! 🙌🏻
Signed-off-by: Pete Lumbis <pete@upbound.io>
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 didn't review closely - the Helm chart README change requires my steering committee stamp. I trust @hasheddan's review.
Backport failed for Please cherry-pick the changes locally. git fetch origin release-1.7
git worktree add -d .worktree/backport-3337-to-release-1.7 origin/release-1.7
cd .worktree/backport-3337-to-release-1.7
git checkout -b backport-3337-to-release-1.7
ancref=$(git merge-base f8020caabbaa34af1eb134a1c28e2e6b0f4a93f4 c0d6d4078c19eebe268280f943b63dc95762a937)
git cherry-pick -x $ancref..c0d6d4078c19eebe268280f943b63dc95762a937 |
Backport failed for Please cherry-pick the changes locally. git fetch origin release-1.8
git worktree add -d .worktree/backport-3337-to-release-1.8 origin/release-1.8
cd .worktree/backport-3337-to-release-1.8
git checkout -b backport-3337-to-release-1.8
ancref=$(git merge-base f8020caabbaa34af1eb134a1c28e2e6b0f4a93f4 c0d6d4078c19eebe268280f943b63dc95762a937)
git cherry-pick -x $ancref..c0d6d4078c19eebe268280f943b63dc95762a937 |
Backport failed for Please cherry-pick the changes locally. git fetch origin release-1.9
git worktree add -d .worktree/backport-3337-to-release-1.9 origin/release-1.9
cd .worktree/backport-3337-to-release-1.9
git checkout -b backport-3337-to-release-1.9
ancref=$(git merge-base f8020caabbaa34af1eb134a1c28e2e6b0f4a93f4 c0d6d4078c19eebe268280f943b63dc95762a937)
git cherry-pick -x $ancref..c0d6d4078c19eebe268280f943b63dc95762a937 |
Description of your changes
The work to migrate crossplane.github.io to Hugo has been completed in the site repo PR #151.
The content of the docs is pushed from /docs into the site repo via Make.
This PR provides the following updates:
/content
)frontmatter
) and content organization expectations. This mainly involves moving files fromOverview.md
to_index.md
at the root of all directories.frontmatter
and our CoC and community guidelines. (indocs/contributing/docs.md
)I also verified that no changes seem to be required to
/build
.Fixes # crossplane/docs#149
This will need to be merged shortly after the site PR is merged.
I have:
make reviewable
to ensure this PR is ready for review.backport release-x.y
labels to auto-backport this PR if necessary.How has this code been tested
Tested through manual verification in a Hugo environment.
The changes to
Makefile
haven't been tested.