-
Notifications
You must be signed in to change notification settings - Fork 131
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
Refresh readme #1078
Refresh readme #1078
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.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: 0 of 1 LGTMs obtained, and 5 discussions need to be resolved
README.md
line 10 at r1 (raw file):
## What's NativeLink? NativeLink is a blazingly fast, high-performance build cache and remote execution system that accelerates software compilation and testing while reducing infrastructure costs. It optimizes build processes for large-scale projects by intelligently caching build artifacts and distributing tasks across multiple machines, delivering exceptional speed improvements. NativeLink powers over one billion requests per month for customers using the system in production workloads.
TBH I'm not a fan of this rewrite.
- It screams "generated by an LLM".
- Its more verbose and more tiring to read.
- Using the term "blazingly" unironically seems weird.
- Blazing speed and high performance are essentially the same thing. We're missing efficiency from this wich is a crucial differentiator, perhaps even more crucial than the actual speed of execution.
- It doesn't only target large-scale projects. Small projects are just as important. Project size doesn't necessarily correlate with complexity and "advanced" build requirements.
- There are more things that aren't exactly correct, but the point is that the previous description (while certainly not perfect) is a better and more accurate description of what NativeLink is.
I propose leaving the start of the readme as-is for now and deferring changes here to another commit.
README.md
line 43 at r1 (raw file):
Choose one of the following methods to install NativeLink: #### 📦 Installing with Cargo
I believe quadruple hashtags don't get rendered as links in the sidebar of the docs.
README.md
line 159 at r1 (raw file):
curl -O https://raw.githubusercontent.com/TraceMachina/nativelink/main/nativelink-config/examples/basic_cas.json ### you can modify the example above to replace the filesystem store with the memory store if you favor speed over data durability.
Single '#'. Capitalize sentence.
With the exeption of links, try to keep the maximum line length below 80. This way code snippets become more readable on mobile devices and it makes the document easier to read and edit with terminal-based editors.
README.md
line 239 at r1 (raw file):
## ✍️ Authors <a href="https://github.com/tracemachina/nativelink/graphs/contributors">
This document is automatically transformed to an mdx file and hosted on https://docs.nativelink.com/tutorials/setup/. The transformer script is likely to not work properly with the angle braces out of the box and might need adjustment https://github.com/TraceMachina/nativelink/blob/main/docs/scripts/md_to_mdx.ts.
README.md
line 246 at r1 (raw file):
## 🤝 Contributing Visit our [Contributing](https://github.com/tracemachina/nativelink/blob/main/CONTRIBUTING.md) guide to learn how to contribute to NativeLink. It only takes ~5 minutes to get started!
The sentence "it only takes 5 minutes to get started" might come off as condescending. If it takes a user more than 5 minutes to get started they shouldn't feel bad about it.
It seems safer to remove the sentence.
@aaronmondal Thanks for these, will wait to change a few of these until installation instructions are migrated as we chatted about. When you get a chance, could you drop a few bullet points on the intro on whats inaccurate/incorrect so I can revise accordingly? |
414761c
to
1ebeace
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.
Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Analyze (javascript-typescript), Analyze (python), Bazel Dev / ubuntu-22.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, Installation / macos-13, Installation / macos-14, Installation / ubuntu-22.04, Local / ubuntu-22.04, Publish image, Publish nativelink-worker-init, Publish nativelink-worker-lre-cc, Remote / large-ubuntu-22.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), macos-13, pre-commit-checks, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, windows-2022 / stable, and 5 discussions need to be resolved
README.md
line 10 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
TBH I'm not a fan of this rewrite.
- It screams "generated by an LLM".
- Its more verbose and more tiring to read.
- Using the term "blazingly" unironically seems weird.
- Blazing speed and high performance are essentially the same thing. We're missing efficiency from this wich is a crucial differentiator, perhaps even more crucial than the actual speed of execution.
- It doesn't only target large-scale projects. Small projects are just as important. Project size doesn't necessarily correlate with complexity and "advanced" build requirements.
- There are more things that aren't exactly correct, but the point is that the previous description (while certainly not perfect) is a better and more accurate description of what NativeLink is.
I propose leaving the start of the readme as-is for now and deferring changes here to another commit.
addressed in our call. Waiting on your specific edits
README.md
line 43 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
I believe quadruple hashtags don't get rendered as links in the sidebar of the docs.
installation being moved to other file -- will be removed in rebase
README.md
line 159 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
Single '#'. Capitalize sentence.
With the exeption of links, try to keep the maximum line length below 80. This way code snippets become more readable on mobile devices and it makes the document easier to read and edit with terminal-based editors.
done.
README.md
line 239 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
This document is automatically transformed to an mdx file and hosted on https://docs.nativelink.com/tutorials/setup/. The transformer script is likely to not work properly with the angle braces out of the box and might need adjustment https://github.com/TraceMachina/nativelink/blob/main/docs/scripts/md_to_mdx.ts.
done.
README.md
line 246 at r1 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
The sentence "it only takes 5 minutes to get started" might come off as condescending. If it takes a user more than 5 minutes to get started they shouldn't feel bad about it.
It seems safer to remove the sentence.
done.
dee3cdb
to
06c5d39
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.
Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Analyze (javascript-typescript), Analyze (python), Bazel Dev / ubuntu-22.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, Installation / macos-13, Installation / macos-14, Installation / ubuntu-22.04, Local / ubuntu-22.04, Publish image, Publish nativelink-worker-init, Publish nativelink-worker-lre-cc, Remote / large-ubuntu-22.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), macos-13, pre-commit-checks, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, windows-2022 / stable, and 5 discussions need to be resolved
README.md
line 10 at r1 (raw file):
Previously, krishmoran (Krish Moran) wrote…
addressed in our call. Waiting on your specific edits
done
README.md
line 43 at r1 (raw file):
Previously, krishmoran (Krish Moran) wrote…
installation being moved to other file -- will be removed in rebase
done.
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.
Reviewable status: 1 of 1 LGTMs obtained, and pending CI: Analyze (python), Bazel Dev / ubuntu-22.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, Installation / macos-13, Installation / macos-14, Installation / ubuntu-22.04, Local / ubuntu-22.04, Publish image, Publish nativelink-worker-init, Publish nativelink-worker-lre-cc, Remote / large-ubuntu-22.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), macos-13, pre-commit-checks, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, windows-2022 / stable, and 5 discussions need to be resolved
06c5d39
to
c90d3cb
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.
Looks good!
I believe there is some rebase issue. The diff looks like it's changing things that are already on main. Could you rebase one more time? As soon as this is resolved we should be fine to merge.
Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: 1 of 1 LGTMs obtained, and 2 discussions need to be resolved
-- commits
line 4 at r4:
Either put a commit message that differs from the title, or probably easier, just remove the message altogether. The title alone is fine for this PR.
README.md
line 10 at r4 (raw file):
</picture> </a> </p>
Is this </p>
intended?
c90d3cb
to
f9625ac
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.
done
Reviewable status: 1 of 1 LGTMs obtained, and pending CI: Analyze (javascript-typescript), Analyze (python), Bazel Dev / ubuntu-22.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, Installation / macos-13, Installation / macos-14, Installation / ubuntu-22.04, Local / ubuntu-22.04, Publish image, Publish nativelink-worker-init, Publish nativelink-worker-lre-cc, Remote / large-ubuntu-22.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), macos-13, pre-commit-checks, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, vale, windows-2022 / stable, and 2 discussions need to be resolved
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
Either put a commit message that differs from the title, or probably easier, just remove the message altogether. The title alone is fine for this PR.
done
README.md
line 10 at r4 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
Is this
</p>
intended?
yes
done
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.
Mentioned a few nits that you might want to double-check before we land. Let me know when you're happy with things and I'll merge.
Reviewable status: 2 of 1 LGTMs obtained, and pending CI: Installation / ubuntu-22.04, Remote / large-ubuntu-22.04, and 4 discussions need to be resolved
Previously, krishmoran (Krish Moran) wrote…
done
Hmm the message body is still here. But it's ok I'll fix it during merge.
README.md
line 10 at r4 (raw file):
Previously, krishmoran (Krish Moran) wrote…
yes
done
Ah this was just displayed confusingly in reviewable.
README.md
line 19 at r4 (raw file):
[![License](https://img.shields.io/badge/License-Apache_2.0-blue.svg)](https://opensource.org/licenses/Apache-2.0) ## What's NativeLink?
nit: Note that this won't have an emoji in the sidebar. (If that's intended, no problem - just letting you know in case it's unintended).
README.md
line 24 at r4 (raw file):
NativeLink is trusted in production environments to reduce costs and developer iteration times--handling over **one billion requests** per month for its customers, including large corporations such as **Samsung**.
nit: remove one newline here (just one, not two)
README.md
line 114 at r4 (raw file):
<img src="https://contrib.rocks/image?repo=tracemachina/nativelink" /> </a>
nit: remove one newline here (just one, not two)
README.md
line 122 at r4 (raw file):
## 📊 Stats ![Alt](https://repobeats.axiom.co/api/embed/d8bfc6d283632c060beaab1e69494c2f7774a548.svg "Repobeats analytics image")
nit: Double-check that the clickability of this image is how you want it. Markdown sometimes acts a bit funky with this stuff. For instance, we have wrappers around the badges at the top of the file because otherwise the links just point to the raw svgs instead of actual links.
Refresh readme
f9625ac
to
13c1866
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, done
Reviewable status: 2 of 1 LGTMs obtained, and pending CI: Cargo Dev / macos-13
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
Hmm the message body is still here. But it's ok I'll fix it during merge.
done
README.md
line 10 at r4 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
Ah this was just displayed confusingly in reviewable.
done
README.md
line 19 at r4 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
nit: Note that this won't have an emoji in the sidebar. (If that's intended, no problem - just letting you know in case it's unintended).
done
README.md
line 24 at r4 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
nit: remove one newline here (just one, not two)
done
README.md
line 114 at r4 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
nit: remove one newline here (just one, not two)
done
README.md
line 122 at r4 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
nit: Double-check that the clickability of this image is how you want it. Markdown sometimes acts a bit funky with this stuff. For instance, we have wrappers around the badges at the top of the file because otherwise the links just point to the raw svgs instead of actual links.
done
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.
Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: 2 of 1 LGTMs obtained, and pending CI: Cargo Dev / macos-13
Description
Updates README for clarity, organization, and contributor engagement:
Content Refinements:
comprehension
Community Focus:
statistics, and how to contribute
product and team
These changes are designed to provide a more user-friendly, informative, and
engaging experience for both new visitors and potential contributors to
NativeLink.
(Visual changes added for better outreach conversion in PR #1074, waiting to be added)
Fixes # (issue)
Type of change
Please delete options that aren't relevant.
Checklist
git amend
see some docsThis change is![Reviewable](https://camo.githubusercontent.com/1541c4039185914e83657d3683ec25920c672c6c5c7ab4240ee7bff601adec0b/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)