Rewrite contribution documentation #827
Conversation
There was a problem hiding this comment.
+@nfarah86 +@blakehatch +@MarcusSorealheis
See https://github.com/aaronmondal/nativelink/blob/rewrite-contribution-docs/CONTRIBUTING.md for a human-readable version.
Reviewable status: 0 of 3 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 (waiting on @blakehatch, @MarcusSorealheis, and @nfarah86)
There was a problem hiding this comment.
Directionally, this seems fine. The only gripe I have is that you have power shell specific instructions but not distinguish when instructions are for unix based systems. That is something you need to do because you call out windows power shell. There are only a few areas.
Reviewable status: 0 of 3 LGTMs obtained, and pending CI: integration-tests (20.04), integration-tests (22.04) (waiting on @blakehatch and @nfarah86)
There was a problem hiding this comment.
I like these changes, especially how detail things like the rebase instructions are. Mostly formatting and some minor clarity changes I believe are needed.
Reviewed all commit messages.
Reviewable status: 0 of 3 LGTMs obtained, and pending CI: integration-tests (20.04), integration-tests (22.04), and 6 discussions need to be resolved (waiting on @MarcusSorealheis and @nfarah86)
CONTRIBUTING.md line 38 at r1 (raw file):
[!WARNING]
Not formatting correctly in human readible form. I believe the carrot has to be indented all the way to the left.
CONTRIBUTING.md line 57 at r1 (raw file):
[!TIP]
Ditto on formatting
CONTRIBUTING.md line 110 at r1 (raw file):
[!NOTE]
Ditto on formatting
CONTRIBUTING.md line 115 at r1 (raw file):
> branches or make changes to the nix files. > [!TIP]
Ditto
CONTRIBUTING.md line 148 at r1 (raw file):
[!TIP]
Ditto
CONTRIBUTING.md line 288 at r1 (raw file):
### Running pre-commit hooks Assuming you're in the nix development environment:
Explain actually how to get into a nix development environment and know whether you're in it. Something like:
Getting in: nix develop
Checking if you're in env
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: 0 of 3 LGTMs obtained, and pending CI: integration-tests (20.04), integration-tests (22.04), and 6 discussions need to be resolved (waiting on @blakehatch, @MarcusSorealheis, and @nfarah86)
aaronmondal
force-pushed
the
rewrite-contribution-docs
branch
2 times, most recently
from
065e535 to
de91260
Compare
There was a problem hiding this comment.
Reviewable status: 0 of 3 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 6 discussions need to be resolved (waiting on @blakehatch, @MarcusSorealheis, and @nfarah86)
CONTRIBUTING.md line 38 at r1 (raw file):
Previously, blakehatch (Blake Hatch) wrote…
Not formatting correctly in human readible form. I believe the carrot has to be indented all the way to the left.
Done.
CONTRIBUTING.md line 57 at r1 (raw file):
Previously, blakehatch (Blake Hatch) wrote…
Ditto on formatting
Done.
CONTRIBUTING.md line 110 at r1 (raw file):
Previously, blakehatch (Blake Hatch) wrote…
Ditto on formatting
Done.
CONTRIBUTING.md line 115 at r1 (raw file):
Previously, blakehatch (Blake Hatch) wrote…
Ditto
Done.
CONTRIBUTING.md line 148 at r1 (raw file):
Previously, blakehatch (Blake Hatch) wrote…
Ditto
Done.
CONTRIBUTING.md line 288 at r1 (raw file):
Previously, blakehatch (Blake Hatch) wrote…
Explain actually how to get into a nix development environment and now how you're in it. Something like:
Getting in:
nix developChecking if you're in
env
Done.
aaronmondal
force-pushed
the
rewrite-contribution-docs
branch
from
de91260 to
02e76a2
Compare
This should help new contributors to get started more easily. The new guidelines are more explicit than the previous ones and go into more detail on common development workflows.
aaronmondal
force-pushed
the
rewrite-contribution-docs
branch
from
02e76a2 to
0d5d845
Compare
There was a problem hiding this comment.
Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: 1 of 3 LGTMs obtained (waiting on @MarcusSorealheis and @nfarah86)
This should help new contributors to get started more easily. The new guidelines are more explicit than the previous ones and go into more detail on common development workflows.
This should help new contributors to get started more easily. The new guidelines are more explicit than the previous ones and go into more detail on common development workflows.
This change is