Page MenuHomePhabricator

[docs] Rearrange "Running keyserver" and "Running web app/landing page"
ClosedPublic

Authored by abosh on Jun 29 2022, 9:05 AM.
Tags
None
Referenced Files
F2191664: D4399.id14049.diff
Thu, Jul 4, 4:44 PM
Unknown Object (File)
Sun, Jun 30, 5:52 AM
Unknown Object (File)
Sat, Jun 29, 6:30 AM
Unknown Object (File)
Fri, Jun 28, 10:02 PM
Unknown Object (File)
Thu, Jun 27, 12:40 AM
Unknown Object (File)
Tue, Jun 25, 6:34 PM
Unknown Object (File)
Tue, Jun 25, 9:49 AM
Unknown Object (File)
Sat, Jun 22, 5:51 AM

Details

Summary

Relevant Linear issue with full context here. Rearranges the "Running keyserver" and "Running web app/landing page" sections and also adds a short sentence about making sure the keyserver is running in the web app, landing page, and mobile app sections. More details in inline comments.


Aside, but viewing docs diffs with the Phabricator diff tool (and really most diff tools) can be kind of a headache since it's a text file, not code. I know we can't control the diff tool on Phabricator, but here are some cool options for diff tools that reduce the amount of green/red you have to parse together to understand what was changed and what remained the same.

Test Plan

N/A

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

docs/dev_environment.md
615 ↗(On Diff #13968)

This style of linking to other sections in the document is not used anywhere else in the docs. I know @ashoat pushes for looking for other examples of things before putting them up, but I stated in the Linear issue description that I wasn't sure the best way to tell the developer to "make sure the keyserver" was already running, so I wrote this as a potential starting point and propose another option below. We could do what we did with the Metro bundler, that is, something like:

Next, make sure that the Metro bundler is running. If you haven't already, open a new terminal and run:

cd native
yarn dev

But since the "make sure the keyserver is running" sentence needs to be at the top of four consecutive sections (web app, landing page, mobile app on iOS Simulator, and mobile app on Android Emulator), I wasn't sure if writing something like this at the top of each section would be overkill:

First, make sure that the keyserver is running. If you haven't already, run:

cd keyserver
yarn dev

Especially since the code to run the keyserver is right above those four sections. Open to discussion about the best way to write this.

624 ↗(On Diff #13968)

This was pushed down from the "Running keyserver" section because it makes more sense here. The web app cannot be viewed in the developer's web browser until:

  1. The keyserver is running
  2. The web app is running

Thus, by moving this statement below the code to run the web app, it now more accurately represents what's running.

639 ↗(On Diff #13968)

This was pushed down from the "Running keyserver" section because it makes more sense here. The landing page cannot be viewed in the developer's web browser until:

  1. The keyserver is running
  2. The landing page is running

Thus, by moving this statement below the code to run the landing page, it now more accurately represents what's running.

671 ↗(On Diff #13968)

This was something I must have missed in D4257, I'm not sure how. Fixed it here because it's a really quick change.

docs/dev_environment.md
609 ↗(On Diff #13968)

This was added to match the similar paragraphs in the below sections. The paragraphs in the "Running web app" and "Running mobile app" sections both say "This command..."

615 ↗(On Diff #13968)

Additionally, we could do something like "(see "Running keyserver" section for details)" which is how it's done in the "Running mobile app on Android Emulator" section:

This command runs two processes (see previous section for details).

ashoat requested changes to this revision.Jun 29 2022, 1:11 PM

Can you please split this diff into two: one that does the move, and another that does the changes? More context here

This revision now requires changes to proceed.Jun 29 2022, 1:11 PM

Can you please split this diff into two: one that does the move, and another that does the changes?

Updated this diff to only include the changes that does the move/rearrange. Will create a new diff that depends on this one to make the changes. Edit: here it is, D4414

This diff just moves the "Running keyserver" section to above "Running web app/landing page."

This revision is now accepted and ready to land.Jun 30 2022, 1:35 PM