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

Refresh readme #1078

Merged
merged 1 commit into from
Jul 11, 2024
Merged

Refresh readme #1078

merged 1 commit into from
Jul 11, 2024

Conversation

krishmoran
Copy link
Contributor

@krishmoran krishmoran commented Jul 4, 2024

Description

Updates README for clarity, organization, and contributor engagement:

Content Refinements:

  • Simplified and expanded the introduction for better clarity and
    comprehension
  • Reorganized "Getting Started" section:
    • Grouped installation instructions separately from build instructions
    • Improved topological organization for a more intuitive user experience

Community Focus:

  • Added new section highlighting open-source contributors, project
    statistics, and how to contribute
  • Aim: Foster community growth and encourage more engagement with the
    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.

  • This change requires a documentation update

Checklist

  • Updated documentation if needed
  • PR is contained in a single commit, using git amend see some docs

This change is Reviewable

@CLAassistant
Copy link

CLAassistant commented Jul 4, 2024

CLA assistant check
All committers have signed the CLA.

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 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.

@krishmoran
Copy link
Contributor Author

@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?

@krishmoran krishmoran force-pushed the readme-refresh branch 3 times, most recently from 414761c to 1ebeace Compare July 8, 2024 21:44
Copy link
Contributor Author

@krishmoran krishmoran 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: 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.

@krishmoran krishmoran force-pushed the readme-refresh branch 2 times, most recently from dee3cdb to 06c5d39 Compare July 10, 2024 20:29
Copy link
Contributor Author

@krishmoran krishmoran 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: 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.

Copy link
Member

@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.

:lgtm:

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

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.

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?

Copy link
Contributor Author

@krishmoran krishmoran left a 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


-- commits line 4 at r4:

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

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.

:lgtm:

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


-- commits line 4 at r4:

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
Copy link
Contributor Author

@krishmoran krishmoran left a 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


-- commits line 4 at r4:

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

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.

:lgtm:

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: 2 of 1 LGTMs obtained, and pending CI: Cargo Dev / macos-13

@aaronmondal aaronmondal enabled auto-merge (squash) July 11, 2024 19:50
@aaronmondal aaronmondal merged commit 414289a into TraceMachina:main Jul 11, 2024
29 checks passed
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.

None yet

4 participants