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

make 'yarn start' be aware of BUILD_OUT_ROOT #6349

Merged
merged 2 commits into from
Jul 21, 2021

Conversation

peterbe
Copy link
Contributor

@peterbe peterbe commented Jun 25, 2021

Fixes #6349

Now, if you first run:

yarn start

in one terminal, and

yarn build

in another. Now both the yarn build and the yarn start will share the same overwritten BUILD_OUT_ROOT environment variable.

@peterbe
Copy link
Contributor Author

peterbe commented Jul 12, 2021

@hamishwillee If you can test this, I'll merge it.

@peterbe
Copy link
Contributor Author

peterbe commented Jul 12, 2021

...unless @fiji-flo gets a chance to review it before you.

@hamishwillee
Copy link
Collaborator

I'm hoping @fiji-flo will get to it first. The queue from being away for 2 weeks is out of control.

@fiji-flo
Copy link
Contributor

@peterbe Looks fine and working but in what scenario does yarn start build something?

@peterbe
Copy link
Contributor Author

peterbe commented Jul 14, 2021

@peterbe Looks fine and working but in what scenario does yarn start build something?

It doesn't. You'd need to run both yarn start and yarn build in two separate terminals. Then the localhost:5000 server can analyze the built files.

@hamishwillee
Copy link
Collaborator

Question (that should not block this going in). What's the best way to make this visible to potential users? The per-page flaw checker is visible on the top of page when you edit a page locally, but how would you find this trick?

@peterbe
Copy link
Contributor Author

peterbe commented Jul 15, 2021

Question (that should not block this going in). What's the best way to make this visible to potential users? The per-page flaw checker is visible on the top of page when you edit a page locally, but how would you find this trick?

There's a link to the "Flaws Dashboard" on the (Writer's) home page.

@hamishwillee
Copy link
Collaborator

So is this now tested?

@peterbe
Copy link
Contributor Author

peterbe commented Jul 16, 2021

So is this now tested?

What do you mean?

If you're asking about this PR; what it does is that it makes it possible to start the "Flaws Dashboard" and get a complete picture of ALL flaws when you build ALL content.

@peterbe
Copy link
Contributor Author

peterbe commented Jul 16, 2021

@hamishwillee To speed up the review of this; can you manually apply this change that this PR does to your local package.json and then do the two separate terminals as described above.
If you're now able to get a complete picture of all flaws with filtering and sorting etc, then let is know that it fully works.

@hamishwillee
Copy link
Collaborator

hamishwillee commented Jul 19, 2021

Hi @peterbe
Sadly, not working for me.

  1. On Windows, synced to your branch, ran yarn install then yarn start. As you can see below this is the version with correct BUILD_OUT_ROOT
    image
  2. Other CMD prompt ran yarn build

I can see it building, but the dashboard at http://localhost:5000/en-US/_flaws is showing zero issues after quite a few minutes.

P.S. What I meant previously about testing was "Is it enough to say I see flaws appearing in the list". If not, what do you need tested about this.

@peterbe
Copy link
Contributor Author

peterbe commented Jul 19, 2021

P.S. What I meant previously about testing was "Is it enough to say I see flaws appearing in the list". If not, what do you need tested about this.

That is correct. I'd be very surprised if there are 0 flaws :)

@peterbe
Copy link
Contributor Author

peterbe commented Jul 19, 2021

Sadly, not working for me.

Not for me either!
Something must have regressed.

@peterbe
Copy link
Contributor Author

peterbe commented Jul 19, 2021

Sadly, not working for me.

Not for me either!
Something must have regressed.

Now I know what it was!
To solve this whole problem, it required/requires 2 separate solutions:

  1. A fix to mdn/yari
  2. A fix to mdn/content

The latter one is this PR. And the fix to mdn/yari was: mdn/yari#4093

So what I did was merged in the latest main into this PR. So when you do yarn install the version of node_modules/@mdn/yari/server/flaws.js works.

In conclusion, @hamishwillee please try again. Check out my latest branch and run yarn install again before you run yarn start.

@hamishwillee
Copy link
Collaborator

It works. A thing of beauty. Get it in :-).

For next iteration, consider providing a visual hint that the page is regenerating when reordering the table. It takes a while on my computer, during which nothing appears to happen. Not a problem except I thought that it was broken.

@peterbe
Copy link
Contributor Author

peterbe commented Jul 21, 2021

I'm going to merge it then. If it works, it works.

@peterbe peterbe merged commit dd1dc75 into mdn:main Jul 21, 2021
@peterbe peterbe deleted the make-yarn-start-be-aware-of-build_out_root branch July 21, 2021 11:13
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants