Skip to content
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

feat: [M3-7538] - Cloud Manager Docs with Vitepress #10027

Merged
merged 13 commits into from
Jan 5, 2024

Conversation

abailly-akamai
Copy link
Contributor

@abailly-akamai abailly-akamai commented Jan 3, 2024

Description 📝

This is the latest iteration of a docsite generation for the Cloud Manager docs.

It iterates over the two previous POCs:

This version addresses the concerns of added dependencies to build the docs and utilizes the much better looking vitepress version. Instead of adding dev dependencies to the monorepo we utilize npx (more specifically its bun equivalent bunx) to get those on the fly during build time.

As a result there's 0 dependencies added to generate our docs 🎉
The same process is used to develop locally.

The pages will eventually live at https://linode.github.io/manager/ but until then the PR has been merged we can see the result at:
👉 https://abailly-akamai.github.io/manager/

Thanks @bnussman-akamai for the vitepress suggestion, GH action and @mjac0bs for the inspiration!

Note: We will need to also link to this documentation from the README and in other knowledge sources, tasks that will be done separately (we also need to clean/update the docs themselves)

Changes 🔄

  • Configure vitepress to build our docs automatically

Preview 📷

Screenshot 2024-01-03 at 14 10 12

How to test 🧪

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@abailly-akamai abailly-akamai changed the title [M3-7538] Vitepress [M3-7538] Cloud Manager Docs with Vitepress Jan 3, 2024
@abailly-akamai abailly-akamai self-assigned this Jan 3, 2024

- uses: oven-sh/setup-bun@v1
with:
bun-version: 1.0.21
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pinning bun version for good measure


- name: Build with VitePress
run: |
bunx vitepress@1.0.0-rc.35 build docs
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pinning the vitepress version as well. We may want to update this when the major version comes out

return markdownInfoArray;
}

export const guides = scanDirectory(DEVELOPMENT_GUIDE_PATH);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a pretty basic plugin to aggregate the pages in the development-guide and populate the left sidebar.

As discussed with @bnussman-akamai it is not super dynamic at the moment since it won't handle recursion or other directories, but it serves our purpose well considering the doc structure isn't something that's been updated (or will be updated) very soon. If it does it wouldn't be too hard to update top get there.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we include this info as a comment (as least: "Aggregates the pages in the development-guide and populates the left sidebar") in this file?

@abailly-akamai abailly-akamai marked this pull request as ready for review January 3, 2024 19:19
@abailly-akamai abailly-akamai requested a review from a team as a code owner January 3, 2024 19:19
@abailly-akamai abailly-akamai requested review from jdamore-linode and mjac0bs and removed request for a team January 3, 2024 19:19

on:
push:
branches: [develop]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we may want to target master eventually? I think it's ok for now but open to feedback

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My vote would be keeping develop with this setup. Our docs are mostly targeted at developers and we want them looking at our latest documentation at all times.

I also hold this opinion on Storybook but I don't think many agree

If we start building multiple environments (like design.dev.linode.com or docs.cloud.dev.linode.com) then it would be a different story imo

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My vote would be keeping develop with this setup. Our docs are mostly targeted at developers and we want them looking at our latest documentation at all times.

I'd agree with this.

Copy link

github-actions bot commented Jan 3, 2024

Coverage Report:
Base Coverage: 79.16%
Current Coverage: 79.16%

Copy link
Member

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome 🎉 I'm happy with the outcome!

This will definitely make it more appealing to document (and consume documentation) for Cloud Manager


on:
push:
branches: [develop]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My vote would be keeping develop with this setup. Our docs are mostly targeted at developers and we want them looking at our latest documentation at all times.

I also hold this opinion on Storybook but I don't think many agree

If we start building multiple environments (like design.dev.linode.com or docs.cloud.dev.linode.com) then it would be a different story imo

.github/workflows/docs.yml Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
docs/plugins/sidebar.ts Outdated Show resolved Hide resolved
@bnussman-akamai bnussman-akamai changed the title [M3-7538] Cloud Manager Docs with Vitepress feat: [M3-7538] - Cloud Manager Docs with Vitepress Jan 3, 2024
Copy link
Contributor

@mjac0bs mjac0bs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple minor comments about documentation. This was looking good in the test deploy and running locally! 🎉


on:
push:
branches: [develop]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My vote would be keeping develop with this setup. Our docs are mostly targeted at developers and we want them looking at our latest documentation at all times.

I'd agree with this.

package.json Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
return markdownInfoArray;
}

export const guides = scanDirectory(DEVELOPMENT_GUIDE_PATH);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we include this info as a comment (as least: "Aggregates the pages in the development-guide and populates the left sidebar") in this file?

@bnussman-akamai bnussman-akamai added Documentation Improving / adding to our documentation Approved Multiple approvals and ready to merge! labels Jan 3, 2024
Copy link
Contributor

@jdamore-linode jdamore-linode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, awesome work @abailly-akamai and @bnussman-akamai! Looking forward to sprucing up the testing docs once this is merged 🎉

What's the motivation behind using bunx over npx? I thought npx was a nice idea since it wouldn't require developers to install anything in addition to what we already require, and it's one more dependency we need support/document/maintain (similarly, I think we should also add a step in GETTING_STARTED.md to instruct the user to install Bun, and document which version(s) we support). Regardless, it is cool seeing Bun in action!

@bnussman-akamai
Copy link
Member

bnussman-akamai commented Jan 4, 2024

@jdamore-linode I generally agree that npx would be nice to not introduce additional dependency. The only reason I'm okay with (and encouraged) bunx here is because I'm really hoping and betting on Bun replacing node for us in over the next few months/years. I figured this would be a nice low friction way to start integrating Bun into our workflow.

If we want to keep is simple and not take the bet on Bun yet, I'm fine with reverting to npx for this, I just really want Bun to work in the long run!

@jdamore-linode
Copy link
Contributor

Thanks @bnussman-akamai, that makes sense to me! I'm happy (and excited) to give Bun a shot. Also just noticed the addition to CONTRIBUTING.md that mentions the Bun installation, so feel free to disregard what I said about GETTING_STARTED.md.

I just really want Bun to work in the long run!

Hear, hear!

Copy link
Contributor

@cpathipa cpathipa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @abailly-akamai!

@abailly-akamai abailly-akamai merged commit cffed4f into linode:develop Jan 5, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge! Documentation Improving / adding to our documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants