-
Notifications
You must be signed in to change notification settings - Fork 222
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
Move guides to new section #2830
Conversation
How to see the preview of this PR?Go to this URL: https://website-git-deploy-preview-mei-16-meili.vercel.app/docs/branch:add-guides-section Credentials to access the page are in the company's password manager as "Docs deploy preview". |
Hey, am I supposed to be able to see the Guides section in the preview? Because it does not appear |
If this is purely a draft and not yet intended to be merged, can I convert it to a draft PR? |
@guimachiavelli Apologies, I converted it to a draft PR ✅ @CaroFG Link preview doesn't work, because this requires toggling something in website repository (see related changes) You can preview changes with this link: https://website-git-feat-enable-docs-guides-meili.vercel.app/docs |
Ok, LGTM but at some point we will need to consider organizing the guides by themes into subsections |
Co-authored-by: CaroFG <[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.
Two main things:
- I'm a bit concerned about the added line to the workflow file. Wouldn't want to mess up our already frail pipeline. I'm terrible with GH actions and etc, so do ping clémentine if you need help assessing this
- The broken links action is failing. Might be related to the previous item
The rest is fairly minor, mostly due diligence
Co-authored-by: gui machiavelli <[email protected]>
Thanks for the reviews. @guimachiavelli I think the action is failing because GitHub is running the action code that is on the That's because this PR also contains the relevant changes in the GitHub action to validate links to guides. It's a bit more work, but if you prefer, I can move those changes to another PR, so we can:
What do you think? |
* replace ndjson.org link * replace ndjson link
I moved the changes related to CI in #2841. If we merge that one first, the CI will pass on this PR. |
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.
lgtm!
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.
LGTM
Why
Move cookbooks, deployment, and front-end pages from
Learn
toGuides
top-level section.How
guides
foldersidebar-config.json
configuration fileredirects.json
to exhaustively list all updated URLssidebar-config.json
configuration tooguides/*
URLsDeployment
This PR needs to be merged before the website's PR that enables the guides section.
This will allow us to get the following deployment process:
Guides
will be temporarily inaccessible (but the site won't break)Guides
section will be activated and its content will be activated