Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(client/env): split CRUD_MODE into WRITER_MODE and DEV_MODE #8383

Merged
merged 7 commits into from
Jun 6, 2023

Conversation

LeoMcA
Copy link
Member

@LeoMcA LeoMcA commented Mar 9, 2023

Currently we place a lot of “local environment” features/setup behind
the CRUD_MODE environment variable. There's some problems with that:

  • It enables a bunch of writer-specific features we really don't need
    as devs, such as index.json reloading every few seconds, and the flaws
    toolbar
  • It isn't enabled on :5042, so testing certain features (such as glean)
    aren't possible on that port, as they require further setup locally
    which isn't done when CRUD_MODE is false

(MP-292)

Currently we place a lot of “local environment” features/setup behind
the CRUD_MODE environment variable. There's some problems with that:

- It enables a bunch of writer-specific features we really don't need
  as devs, such as index.json reloading every few seconds, and the flaws
  toolbar
- It isn't enabled on :5042, so testing certain features (such as glean)
  aren't possible on that port, as they require further setup locally
  which isn't done when CRUD_MODE is false
@github-actions github-actions bot added github-actions plus work around features related to MDN Plus labels Mar 9, 2023
@LeoMcA LeoMcA marked this pull request as ready for review March 9, 2023 19:48
@LeoMcA LeoMcA requested a review from caugner March 9, 2023 19:48
Copy link
Contributor

@caugner caugner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just 1-2 nits.

client/src/document/toolbar/flaws.tsx Outdated Show resolved Hide resolved
client/src/env.ts Outdated Show resolved Hide resolved
client/src/env.ts Show resolved Hide resolved
client/src/env.ts Outdated Show resolved Hide resolved
client/src/env.ts Outdated Show resolved Hide resolved
@caugner
Copy link
Contributor

caugner commented Mar 10, 2023

@LeoMcA How about prefixing this PR with enhance(client/env) instead of chore?

@LeoMcA LeoMcA changed the title chore: split CRUD_MODE env var into WRITER_MODE and DEV_MODE enhance(client/env): split CRUD_MODE env var into WRITER_MODE and DEV_MODE Mar 10, 2023
@LeoMcA LeoMcA changed the title enhance(client/env): split CRUD_MODE env var into WRITER_MODE and DEV_MODE enhance(client/env): split CRUD_MODE into WRITER_MODE and DEV_MODE Mar 10, 2023
@LeoMcA LeoMcA requested a review from caugner March 10, 2023 17:55
@github-actions github-actions bot added the merge conflicts 🚧 Please rebase onto or merge the latest main. label Mar 15, 2023
@github-actions
Copy link
Contributor

This pull request has merge conflicts that must be resolved before it can be merged.

@caugner caugner marked this pull request as draft April 4, 2023 19:08
Copy link
Contributor

@caugner caugner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs merge conflict resolution. Will look at it again afterwards.

@github-actions github-actions bot removed the merge conflicts 🚧 Please rebase onto or merge the latest main. label May 15, 2023
@LeoMcA LeoMcA marked this pull request as ready for review May 15, 2023 16:12
@LeoMcA LeoMcA requested a review from caugner May 15, 2023 16:12
@caugner caugner changed the title enhance(client/env): split CRUD_MODE into WRITER_MODE and DEV_MODE feat(client/env): split CRUD_MODE into WRITER_MODE and DEV_MODE May 30, 2023
Copy link
Contributor

@caugner caugner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, although we might want to redirect from http://localhost:3000/ to http://localhost:3000/en-US/ if REACT_APP_WRITER_MODE is disabled.

client/src/env.ts Outdated Show resolved Hide resolved
client/src/env.ts Outdated Show resolved Hide resolved
@LeoMcA LeoMcA merged commit 675a854 into main Jun 6, 2023
@LeoMcA LeoMcA deleted the crud-to-writer-dev-mode branch June 6, 2023 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
github-actions internal plus work around features related to MDN Plus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants