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

Add docs to monorepo #453

Merged
merged 1 commit into from
Dec 12, 2023
Merged

Conversation

blakehatch
Copy link
Contributor

@blakehatch blakehatch commented Dec 7, 2023

Description

Adds Mintlify docs to monorepo

Fixes #452

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Checklist

  • Updated documentation if needed
  • Tests added/amended
  • bazel test //... passes locally
  • PR is contained in a single commit, using git amend see some docs

This change is Reviewable

Copy link
Member

@aaronmondal aaronmondal left a comment

Choose a reason for hiding this comment

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

TBH I have quite a few issues with this PR.

"Easily fixable":

  1. Easy to fix, but this should not be a submodule. Just copy-paste the entire thing so that we just need to maintain the git history of a single repo.
  2. From what I tested the current site fails all lighthouse tests. That is, performance on Mobile is mediocre, accessibility criteria aren't met and SEO best practices are not followed. Without frameworks like qwik it's hard to ace those and performance scores are dependent on the edge network (AFAIK cloudflare is the fastest), but acing accessibility should be seen as a hard requirement and acing SEO seems reasonable as well.
  3. It would be nice to have deployment pipelines and CI in place from the start, for instance https://github.com/marketplace/actions/lighthouse-ci-action for performance testing and https://github.com/cloudflare/pages-action or https://vercel.com/guides/how-can-i-use-github-actions-with-vercel for deployments. I think it should be fine to just make the doc repo public if that makes it easier to test different deployment strategies - there isn't too much content in the ATM anyways.

"Not so easy to fix":

  1. The contents of the docs (in their current internal state) are outdated and should instead reference the existing markdown files in the repository. It's bad DX if we need to remember updating documentation in two places and this will inevitably lead to out-of-sync documentation.
  2. I'm not sold on mintlify. I'd say the most important feature for us is that we can generate most of the documentation automatically and have an ecosystem of tooling available that allows (doc) code generation so that we can implement pipelines that directly generate the docs from markdown files and rustdoc. Mintlify doesn't seem to provide either one and would require us to implement this kind of code generation ourselves, which is an extremely longwieldy and tedious task.
  3. Mintlify doesn't have much of a community going on and I'd flag this as a very risky use of new tech that doesn't seem to solve any new issues that existing generators have.
  1. The logo that is currently in the internal repo is uncomfortably similar to astro's logo: https://astro.build/. The similar colorscheme emphasizes this even more. What happened to the soothing dark green scheme with the robot-arm we had before?

I think it would make sense to step back and reevaluate other frameworks. We don't need a website - if we did we could just roll our own stuff with qwik/mitosis/partytown and get much better UX, DX performance and SEO introspection. But what we need from a doc site is seamless integration with development workflows that gets "out of the way" of developers and doesn't require us to effectively maintain two separate documentation codebases.

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @MarcusSorealheis)

Copy link
Member

@aaronmondal aaronmondal left a comment

Choose a reason for hiding this comment

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

After more thinking and researching it seems to me that Docusaurus (https://docusaurus.io/) is the lowest-risk choice.

As migration path we can then switch to Astro (https://docs.astro.build/de/guides/migrate-to-astro/from-docusaurus/) in the future and can swap components to qwik over time (https://qwik.builder.io/docs/integrations/astro/) for further optimization (for instance, aided by mitosis (https://github.com/BuilderIO/mitosis)).

This way we can start off with something simple, mature and stable, but keep the flexibility to migrate to more modern approaches in the future.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @MarcusSorealheis)

Copy link
Contributor Author

@blakehatch blakehatch left a comment

Choose a reason for hiding this comment

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

These are good concerns to bring up. The biggest deal-breaker that would make the decision simple is if we want to improve lighthouse scores we will need to move off of Mintlify, it's a highly abstracted framework that's quick to develop on but offers no way to optimize further than it already is. Also the ecosystem of tooling allowing us to not have to maintain documentation in two separate places between the internal docs and the website.

I'm happy to re-build this in something like docusaurus and if we decide that's something worth doing it's probably best to do at this early stage while the docs are lean.

As far as the logo I'm personally not too concerned as the rocket taking off is such a common design element but it's an understandable worry.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @MarcusSorealheis)

@MarcusSorealheis
Copy link
Collaborator

I think we can just close this PR and restart unless I missing something.

Copy link
Member

@aaronmondal aaronmondal left a comment

Choose a reason for hiding this comment

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

Reviewed 10 of 10 files at r2, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @blakehatch and @MarcusSorealheis)


native-link-docs/favicon.svg line 21 at r2 (raw file):

            c0.067-0.796-0.271-1.47-0.637-2.266C7.199,1.995,6.86,2.669,6.928,3.465L6.323,3.926v0.582H5.042L7.565,0L10.018,4.508z"/>
    <circle fill="#FFFFFF" cx="7.565" cy="2.782" r="0.281"/>
</svg>

Missing newline. The pre-commit hooks in the nix flake should autoformat this for you.


native-link-docs/introduction.mdx line 17 at r2 (raw file):

/>

Native Link is a high-performance project crafted in Rust, designed to serve as a Bazel cache service (CAS) that adheres to the [Remote Execution protocol](https://github.com/bazelbuild/remote-apis). This means it efficiently stores and manages build artifacts, ensuring stability, speed, and low resource usage.

Section out of date with current readme.


native-link-docs/quickstart.mdx line 6 at r2 (raw file):

---

## Setup your development

Section out of sync with current readme.

I see that you made some customizations to the previous readme though, so let me comment on those changes as well:

  1. Users on MacOS shouldn't be using GCC. They should use Clang as C++ toolchain instead which is supports MacOS natively.
  2. There is no need for custom setups since we have the nix flake.
  3. Don't use apt in any installation commands as it's not cross-platform. These commands won't work on non-debian distros and don't even work on all Ubuntu systems. They're also irreproducible, meaning that on e.g. Ubuntu 18 you'll get a toolchain that can't build our project while on Ubuntu 22 it'll work. These kinds of issues are incredibly frustrating to users, so let's prevent this and just recommend the flake which is the exact same environment regardless of whether you run it on Ubuntu 18, upstream Gentoo or MacOS.

native-link-docs/README.md line 1 at r2 (raw file):

# Mintlify Starter Kit

Let's use our own readme here.

Copy link
Member

@adam-singer adam-singer left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @aaronmondal, @blakehatch, and @MarcusSorealheis)


native-link-docs/quickstart.mdx line 6 at r2 (raw file):

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

Section out of sync with current readme.

I see that you made some customizations to the previous readme though, so let me comment on those changes as well:

  1. Users on MacOS shouldn't be using GCC. They should use Clang as C++ toolchain instead which is supports MacOS natively.
  2. There is no need for custom setups since we have the nix flake.
  3. Don't use apt in any installation commands as it's not cross-platform. These commands won't work on non-debian distros and don't even work on all Ubuntu systems. They're also irreproducible, meaning that on e.g. Ubuntu 18 you'll get a toolchain that can't build our project while on Ubuntu 22 it'll work. These kinds of issues are incredibly frustrating to users, so let's prevent this and just recommend the flake which is the exact same environment regardless of whether you run it on Ubuntu 18, upstream Gentoo or MacOS.

opinion:

I think popularizing nix flake is a good thing, maybe that can be in the header section of this document as one way to bootstrap. That being said I don't think it is very mainstream. It could be a turn off if thats the only instructions they see to bootstrap / run tests of this project. One way to look at it, someone is having to research this project for work related requirements, usually have a work issued laptop/environment, generally introducing non-standard (or commonly known) tooling add a layer of intimidation / fear. I would personally in work environments take the apt commands over nix on local or development machines because they would be familiar and commonly seen with other setups. If I was working on my personal computer, I'd probably use nix as another fun tool to try out, messing up a personal laptop doesn't come with the same type of associated risk as a work laptop environment (assuming we are talking about local machines).

For each of the sections on this doc, I would also cross link to the general owning tools websites that provide bootstrap docs, https://rustup.rs/ / https://bazel.build/install/bazelisk / etc.. it is nice to have quick copy and paste in case something is missing, but these things are usually so commonly available on most systems that the point of documentation here is to know you need them (if you don't have them already) vs installing them.

Copy link
Member

@aaronmondal aaronmondal left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @adam-singer, @blakehatch, and @MarcusSorealheis)


native-link-docs/quickstart.mdx line 6 at r2 (raw file):

Previously, adam-singer (Adam Singer) wrote…

opinion:

I think popularizing nix flake is a good thing, maybe that can be in the header section of this document as one way to bootstrap. That being said I don't think it is very mainstream. It could be a turn off if thats the only instructions they see to bootstrap / run tests of this project. One way to look at it, someone is having to research this project for work related requirements, usually have a work issued laptop/environment, generally introducing non-standard (or commonly known) tooling add a layer of intimidation / fear. I would personally in work environments take the apt commands over nix on local or development machines because they would be familiar and commonly seen with other setups. If I was working on my personal computer, I'd probably use nix as another fun tool to try out, messing up a personal laptop doesn't come with the same type of associated risk as a work laptop environment (assuming we are talking about local machines).

For each of the sections on this doc, I would also cross link to the general owning tools websites that provide bootstrap docs, https://rustup.rs/ / https://bazel.build/install/bazelisk / etc.. it is nice to have quick copy and paste in case something is missing, but these things are usually so commonly available on most systems that the point of documentation here is to know you need them (if you don't have them already) vs installing them.

Agreed that it can be daunting to install nix. It's just one command, but that can be off-putting.

On the other hand, I'd also never recommend doing anything with apt though, since I've spent literal days of my life helping users figure out that their toolchains were too old because they were using outdated OSes. I'd also argue that users more open to trying out new things tend to also have custom distros like Arch, Gentoo and void. From personal experience it's often quite annoying if installation commands don't actually contain all the dependencies necessary. For instance, on Ubuntu the clang package will also make some libcxx available. On Gentoo that's not the case and the installation of a full toolchain requires more than one package. Translating commands like that is poor UX.

The way to go might actually be neither: We have prebuilt images, so we could just tell users to use those via docker run ghcr.io/tracemachina/native-link:xxx. That's something that should work for everyone and doesn't require any additional tooling besides docker which we can everyone to know how to install.

The other dependencies are technically only required to build from source. I'd argue that those building from source will be open to more exotic tooling as it's common that every project uses custom build infra and users building from source will generally be aware that they'll need to adjust to the project's build system.

I think it makes sense to have two sections. One using just something like

git clone xxx && \
  cd xxx && \
  docker run ghcr.io/xxx \
    -v native-link-config/examples/basic_cas.json:/config.json \
    -p 50051:50051 \
    -p 50052:50052 \
    /config.json

for ease of use and one "Building from source" section that contains more info on the more elaborate setups.

Copy link
Contributor Author

@blakehatch blakehatch left a comment

Choose a reason for hiding this comment

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

Gonna keep open, was able to fix the github submodule issue.

Reviewable status: 6 of 12 files reviewed, 4 unresolved discussions (waiting on @aaronmondal and @MarcusSorealheis)


native-link-docs/favicon.svg line 21 at r2 (raw file):

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

Missing newline. The pre-commit hooks in the nix flake should autoformat this for you.

Done.


native-link-docs/introduction.mdx line 17 at r2 (raw file):

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

Section out of date with current readme.

Updated to match


native-link-docs/quickstart.mdx line 6 at r2 (raw file):

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

Agreed that it can be daunting to install nix. It's just one command, but that can be off-putting.

On the other hand, I'd also never recommend doing anything with apt though, since I've spent literal days of my life helping users figure out that their toolchains were too old because they were using outdated OSes. I'd also argue that users more open to trying out new things tend to also have custom distros like Arch, Gentoo and void. From personal experience it's often quite annoying if installation commands don't actually contain all the dependencies necessary. For instance, on Ubuntu the clang package will also make some libcxx available. On Gentoo that's not the case and the installation of a full toolchain requires more than one package. Translating commands like that is poor UX.

The way to go might actually be neither: We have prebuilt images, so we could just tell users to use those via docker run ghcr.io/tracemachina/native-link:xxx. That's something that should work for everyone and doesn't require any additional tooling besides docker which we can everyone to know how to install.

The other dependencies are technically only required to build from source. I'd argue that those building from source will be open to more exotic tooling as it's common that every project uses custom build infra and users building from source will generally be aware that they'll need to adjust to the project's build system.

I think it makes sense to have two sections. One using just something like

git clone xxx && \
  cd xxx && \
  docker run ghcr.io/xxx \
    -v native-link-config/examples/basic_cas.json:/config.json \
    -p 50051:50051 \
    -p 50052:50052 \
    /config.json

for ease of use and one "Building from source" section that contains more info on the more elaborate setups.

I decided to go with apt while including Nix in an "Advanced Setup" section. From my understanding most of our users will be debian-based with context switching being fairly simple from apt to other package managers however I can include instructions for others (Pacman, etc.) if there is a compelling reason. Seems like a lot of the users in alternative Distros will prefer/be comfortable using Nix.


native-link-docs/README.md line 1 at r2 (raw file):

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

Let's use our own readme here.

Modified to just include necessary instructions for local development and changed header.

Copy link
Member

@aaronmondal aaronmondal left a comment

Choose a reason for hiding this comment

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

Reviewed 6 of 6 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @blakehatch and @MarcusSorealheis)


native-link-docs/quickstart.mdx line 26 at r3 (raw file):

* Bazel 6.4.0+
* gcc (exc. MacOS)

I'd recommend clang instead of gcc/g++. That's what rust uses under the hood (and where LLD comes from as well https://github.com/llvm/llvm-project/tree/main/lld) and works on Linux, MacOS and Windows.


native-link-docs/quickstart.mdx line 227 at r3 (raw file):

  completely clean and aligned with the commit you want to reproduce. 
  Otherwise the image will be tainted with a `"dirty"` revision label. 
</Tip>

nit: Newline.

Copy link
Member

@aaronmondal aaronmondal left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @blakehatch and @MarcusSorealheis)

@blakehatch blakehatch merged commit 378b806 into TraceMachina:main Dec 12, 2023
19 of 21 checks passed
blakehatch added a commit to blakehatch/nativelink that referenced this pull request Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Docs Folder to Monorepo
4 participants