Update README for More Clarity

[nfarah86]

Description

added mac os warning

Fixes #729

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 

[aaronmondal]

@nfarah86 Since this is doing more than just adding the warning section, could you split this into two PRs?

FYI the stylechecker warnings are visualized in the regular GitHub review view here: https://github.com/TraceMachina/nativelink/pull/803/files

The config for that checker is here: https://github.com/TraceMachina/nativelink/blob/main/.vale.ini

To run the checks manually:

• Set up nix and flakes (for instance with this installer: https://github.com/NixOS/experimental-nix-installer)
• Run nix develop in the repository.
• (Alternatively, install direnv with e.g. nix profile install nixpkgs#direnv and hook it into your shell like https://direnv.net/docs/hook.html. This way the flake automatically activates when you cd into the repository).
• Run pre-commit run -a. If you're in the nix flake this will also run automatically every time you create a commit and will stop the commit if it doesn't pass the pre-commit CI locally.

Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Bazel Dev / ubuntu-22.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), macos-13, pre-commit-checks, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, windows-2022 / stable, zig-cc ubuntu-20.04, zig-cc ubuntu-22.04

[nfarah86]

Hey @aaronmondal is it ok to change the commit messages to reflect the macos warning and that I updated the readme for more clarity? I setup nix and ran pre commit hooks.

Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Bazel Dev / ubuntu-22.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, Local / ubuntu-22.04, Publish image, 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, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, windows-2022 / stable, zig-cc ubuntu-20.04, zig-cc ubuntu-22.04

[aaronmondal]

Sure. Changes to the commit title and message are part of the review in reviewable.

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: 0 of 1 LGTMs obtained, and 2 discussions need to be resolved

---

-- commits line 2 at r2:
If you edit this message while doing a git commit --amend it'll show up here as a change.

---

README.md line 78 at r2 (raw file):

and `worker`.

<!-- vale off -->

Let's not turn of vale unless it's absolutely necessary.

If it raises a false positive because it doesn't know a valid word you can add it to the vocabulary here: https://github.com/TraceMachina/nativelink/blob/main/.github/styles/config/vocabularies/TraceMachina/accept.txt

[nfarah86]

Updated - how does it look now?

Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Vercel, pre-commit-checks, and 2 discussions need to be resolved

[blakehatch]

Reviewed 1 of 2 files at r3, 1 of 1 files at r5, all commit messages.
Reviewable status: 1 of 1 LGTMs obtained, and 2 discussions need to be resolved

---

-- commits line 2 at r2:

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

If you edit this message while doing a git commit --amend it'll show up here as a change.

Or git commit --amend -m "..." so you don't have to exit vim 😂

[aaronmondal]

Reviewed all commit messages.
Reviewable status: 1 of 1 LGTMs obtained, and 2 discussions need to be resolved

---

-- commits line 2 at r2:

Previously, blakehatch (Blake Hatch) wrote…

Or git commit --amend -m "..." so you don't have to exit vim 😂

With -m you'd lose the original message. I.e. the diff between messages wouldn't really be a diff but something else every time you have some larger commit. Also, neovim automatically color-codes too long commit titles and auto-linebreaks the commit message body 🤓

@nfarah86 The format we use for commits is

1. Capitalized title
2. No trailing period
3. Short and concise, but precise title, if possible shorter than 72 characters
4. Details in the body (also limited to 72 characters except for links) if necessary.
5. Imperative tone
6. If the message contains and, it's probably something that should be split into two commits

You can see examples when you run git log in a terminal.

The relevant part here is that a git log invocation should be readable on a terminal. Most editors auto-linebreak so in those cases you wouldn't notice issues with extremely long lines, but when you use a terminal on a server it gives much better user experience. The guidelines are fairly "default", but AFAIK we don't have it documented anywhere (which we should probably change).

[aaronmondal]

Reviewable status: 1 of 1 LGTMs obtained, and 2 discussions need to be resolved

---

README.md line 78 at r2 (raw file):

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

Let's not turn of vale unless it's absolutely necessary.

If it raises a false positive because it doesn't know a valid word you can add it to the vocabulary here: https://github.com/TraceMachina/nativelink/blob/main/.github/styles/config/vocabularies/TraceMachina/accept.txt

fyi @nfarah86 you can Type Done in this message to resolve it or click on the Resolve button to unblock this comment.

[nfarah86]

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, Local / ubuntu-22.04, Publish image, 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, zig-cc ubuntu-20.04, zig-cc ubuntu-22.04, and 2 discussions need to be resolved

---

-- commits line 2 at r2:

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

With -m you'd lose the original message. I.e. the diff between messages wouldn't really be a diff but something else every time you have some larger commit. Also, neovim automatically color-codes too long commit titles and auto-linebreaks the commit message body 🤓

@nfarah86 The format we use for commits is

1. Capitalized title
2. No trailing period
3. Short and concise, but precise title, if possible shorter than 72 characters
4. Details in the body (also limited to 72 characters except for links) if necessary.
5. Imperative tone
6. If the message contains and, it's probably something that should be split into two commits

You can see examples when you run git log in a terminal.

The relevant part here is that a git log invocation should be readable on a terminal. Most editors auto-linebreak so in those cases you wouldn't notice issues with extremely long lines, but when you use a terminal on a server it gives much better user experience. The guidelines are fairly "default", but AFAIK we don't have it documented anywhere (which we should probably change).

Done.

---

README.md line 78 at r2 (raw file):

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…

fyi @nfarah86 you can Type Done in this message to resolve it or click on the Resolve button to unblock this comment.

Done.

[nfarah86]

Reviewable status: 1 of 1 LGTMs obtained, and 2 discussions need to be resolved

README.md line 78 at r2 (raw file):

Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
fyi @nfarah86 you can Type Done in this message to resolve it or click on the Resolve button to unblock this comment.

The resolve button is not showing up for me :(

[aaronmondal]

Reviewed all commit messages.
Reviewable status: complete! 2 of 1 LGTMs obtained