Page MenuHomePhabricator
Differential D5652

[Nix Docs] Mention yarn cleaninstall and git clone
ClosedPublic
Actions

Authored by • jon on Nov 16 2022, 9:29 AM.
Tags
None
Referenced Files
F2896627: D5652.diff
Fri, Oct 4, 8:22 PM
Unknown Object (File)
Thu, Sep 26, 12:39 PM
Unknown Object (File)
Tue, Sep 17, 2:38 AM
Unknown Object (File)
Tue, Sep 17, 2:38 AM
Unknown Object (File)
Tue, Sep 17, 2:38 AM
Unknown Object (File)
Tue, Sep 17, 2:38 AM
Unknown Object (File)
Tue, Sep 17, 2:38 AM
Unknown Object (File)
Tue, Sep 17, 2:30 AM
View All Files
Subscribers
• abosh
atul
tomek

Details

Reviewers
ashoat
varun
Commits
rCOMM5c199c8adba2: [Nix Docs] Mention yarn cleaninstall and git clone
Summary

Ensure that developers have the repository pulled down and that
yarn cleaninstall is ran as part of a development workflow.

https://linear.app/comm/issue/ENG-2242

Test Plan

N/A

Diff Detail

Repository
rCOMM Comm
Branch
jonringer/nix-docs-improvements
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

• jon created this revision.Nov 16 2022, 9:29 AM
Herald added subscribers: • abosh, atul, tomek. · View Herald TranscriptNov 16 2022, 9:29 AM
Harbormaster completed remote builds in B13484: Diff 18491.Nov 16 2022, 9:48 AM
• jon requested review of this revision.Nov 16 2022, 9:48 AM
ashoat requested changes to this revision.Nov 16 2022, 1:03 PM
ashoat added inline comments.
docs/nix_dev_env.md
37 ↗(On Diff #18491)

Capitalize Git

68–75 ↗(On Diff #18491)

I think it's kinda weird to have this mentioned in multiple sections... it might make people think they have to run it twice, or have to run it before each time they use the workflow or something. What do you think of the suggested edit?

75 ↗(On Diff #18491)

Capitalize yarn

docs/nix_mobile_workflows.md
5 ↗(On Diff #18491)

Capitalize React Native

6 ↗(On Diff #18491)

Get rid of line break

docs/nix_web_workflows.md
3–10 ↗(On Diff #18491)

Same as above

This revision now requires changes to proceed.Nov 16 2022, 1:03 PM
• jon updated this revision to Diff 18501.Nov 16 2022, 2:30 PM
• jon marked 4 inline comments as done.

Unify yarn cleaninstall section in main document

• jon marked 2 inline comments as done.Nov 16 2022, 2:31 PM

Moved to the two separate sections into the main document, and just had it follow installing nix

docs/nix_dev_env.md
68–75 ↗(On Diff #18491)

I'm not entirely sure.

My mindset with this, was that the web and mobile workflows were separate. But just adding after the nix install also makes sense.

Harbormaster completed remote builds in B13493: Diff 18501.Nov 16 2022, 2:46 PM
ashoat requested changes to this revision.Nov 16 2022, 8:10 PM
ashoat added inline comments.
docs/nix_dev_env.md
47 ↗(On Diff #18501)

Capitalize Yarn

49 ↗(On Diff #18501)

I prefer my language from the last review. Don't think we need to call out React Native specifically

This revision now requires changes to proceed.Nov 16 2022, 8:10 PM
• jon updated this revision to Diff 18539.Nov 17 2022, 11:00 AM
• jon marked 3 inline comments as done.

Address feedback

docs/nix_dev_env.md
49 ↗(On Diff #18501)

sounds fine to me.

ashoat accepted this revision.Nov 17 2022, 11:07 AM
This revision is now accepted and ready to land.Nov 17 2022, 11:07 AM
Harbormaster completed remote builds in B13526: Diff 18539.Nov 17 2022, 11:13 AM
Closed by commit rCOMM5c199c8adba2: [Nix Docs] Mention yarn cleaninstall and git clone (authored by • jon). · Explain WhyNov 17 2022, 11:44 AM
This revision was automatically updated to reflect the committed changes.
• jon added a commit: rCOMM5c199c8adba2: [Nix Docs] Mention yarn cleaninstall and git clone.

Revision Contents
Changeset List

PathSize
docs/
15 lines

Diff 18539

View Options

docs/nix_dev_env.md

Loading...