Update documentation around Apache as it's no longer necessary
for local development.
Details
Details
- Reviewers
ashoat varun - Commits
- rCOMM8d61beb7ec1a: [Docs] Remove apache configuration
Steps:
- Launch simulator
- Delete existing Comm app (Hold press on application, then select "Remove App", confirm with "Delete App")
- Update commapp_url.json and landing_url.json as described in diff (or delete them, and run nix develop)
- Restart yarn dev from native/, and yarn-dev from keyserver/
- Run simulator again
Rendered docs: https://github.com/CommE2E/comm/blob/jonringer/squadcal-apache/docs/dev_environment.md#urls
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
Not requesting changes because I'm about to go out, but please address this feedback before landing
| docs/dev_environment.md | ||
|---|---|---|
| 459 | Please make this look like the rest. You should always, always try to follow existing language when editing docs. Putting a command directly in like this instead of an English sentence, and not even wrapping it in a code block or inline code element is clearly a huge departure from the style in the docs. When you are editing the docs, YOU need to think of yourself as the editor of a technical doc. Part of an Editor's job is to maintain consistent style. Please think of this going forward, and please integrate this feedback into your workflow so I don't need to act as the team's only Editor. We need everybody on the team to be thoughtful about this | |
| docs/dev_environment.md | ||
|---|---|---|
| 459 | Yep, that was my bad. Should have double checked diff before submitting. | |
| docs/dev_environment.md | ||
|---|---|---|
| 435 ↗ | (On Diff #19289) | we're sure this shouldn't be "/comm/"? |
| docs/dev_environment.md | ||
|---|---|---|
| 459–475 ↗ | (On Diff #19317) | Actually, not even clear why we need this part. The use of squadcal_url.json on dev is only necessary for handling the situation where an existing dev has a "sticky" urlPrefix set in the Redux store of the Comm app on a simulator/emulator, right? Somebody running through the dev_environment.md doc on their first go shouldn't have this issue... they'll get the new port-3000 urlPrefix from D5846 |
| docs/dev_environment.md | ||
|---|---|---|
| 459–475 ↗ | (On Diff #19317) | I updated the linear task associated with this work. https://linear.app/comm/issue/ENG-2459 |
Comment Actions
We should very rarely have "N/A" as the Test Plan. Can you amend the Test Plan to mention testing that these configs of commapp_url.json and landing_url.json work?
Comment Actions
Back to you for Test Plan. Doesn't have to be "installing everything from scratch", but should be more than "N/A"
Comment Actions
We should very rarely have "N/A" as the Test Plan. Can you amend the Test Plan to mention testing that these configs of commapp_url.json and landing_url.json work?
Updated test plan with something which should work outside or inside of nix develop. The original revision was part of the migration stack, so the logic was validated elsewhere (got apache running on m1, validating workflow now).
Since this aimed at new developers which don't have an existing Comm app on their simulator, I think it's reasonable to assume a clean slate.