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

fix(docs): missing and incorrect scripts #43209

Merged
merged 4 commits into from
Aug 8, 2024

Conversation

Jay-Karia
Copy link
Contributor

Closes #43208

Adds a missing script: pnpm install.
Updates an incorrect script: pnpm start to pnpm dev

Signed-off-by: Jay <jay.sanjay.karia@gmail.com>
Signed-off-by: Jay <jay.sanjay.karia@gmail.com>
@mui-bot
Copy link

mui-bot commented Aug 6, 2024

Netlify deploy preview

https://deploy-preview-43209--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 7a81099

@zannager zannager added the docs Improvements or additions to the documentation label Aug 7, 2024
@zannager zannager requested a review from Janpot August 7, 2024 15:07
@Janpot
Copy link
Member

Janpot commented Aug 7, 2024

We don't have a dev script as far as I can see. The start script starts the docs in dev mode as it calls docs:dev under the hood.

@Janpot Janpot mentioned this pull request Aug 7, 2024
1 task
@Jay-Karia
Copy link
Contributor Author

You have referred the package.json file in the root.
I am talking about the package.json file from docs/ directory, where there is dev and start scripts.

@Jay-Karia
Copy link
Contributor Author

So we need to update the README of /docs directory.

@Janpot
Copy link
Member

Janpot commented Aug 7, 2024

So we need to update the README of /docs directory.

That README says to run it from the project root:

To start the docs site in development mode, from the project root, run:

It's confusing though. I'd probably just say pnpm docs:dev everywhere instead of pnpm start

@Jay-Karia
Copy link
Contributor Author

Yeah,
It should be obvious to mention scripts in /docs README that would run in /docs directory instead in root.
This might reduce confusion.

At the end it's upto you.
Will respect your decision. 👍

Signed-off-by: Jay <jay.sanjay.karia@gmail.com>
@Janpot
Copy link
Member

Janpot commented Aug 7, 2024

I'm ok with recommending pnpm docs:dev instead of pnpm start from the root, but not with recommending running scripts from the docs folder. There is no good reason to run scripts in workspaces directly.

Signed-off-by: Jay <jay.sanjay.karia@gmail.com>
@Jay-Karia
Copy link
Contributor Author

Now it's pnpm docs:dev

Copy link
Member

@Janpot Janpot left a comment

Choose a reason for hiding this comment

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

Ok for me. The main problem with the pnpm start script is that it will try to run the docs site in production when run from the wrong folder. At least now the error will say "Command "docs:dev" not found" if run from the docs folder. Improved DX for first time contributors 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docs(readme): incorrect and missing pnpm scripts
4 participants