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

docs: Migrate docs pm to npm from yarn #67

Merged
merged 4 commits into from
Dec 6, 2023

Conversation

kevintyj
Copy link
Contributor

@kevintyj kevintyj commented Dec 6, 2023

What changed / motivation ?

I noticed that the project was mainly using npm, however, the docs app (website) was using yarn. I have edited the package.json to migrate yarn commands to npm and updated README to reflect the changes

Linked PR/Issues

None

Additional Context

  • I would propose migrating to pnpm
  • Also added documentation about building stylex packages to README
  • Important!: It seems that stylelint fails due to a missing package stylelint-copyright and lint & ci commands seems to fail. Was not sure if this was expected behavior and if the linter is only ran on ci.
  • Testing was done to make sure current behavior does not change
  • Added **/build/** to stylelint (was not sure if this is a desired behavior)

Pre-flight Checklist

  • I have read the contributing guidelines
  • I have signed the contributing agreement
  • I have written unit tests where necessary (If it is a feature commit)
  • Performed a self-review of my code
  • Made sure PR title follows the conventional commit specification

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 6, 2023
@kevintyj kevintyj changed the title Chore/docs to npm docs: Migrate docs pm to npm from yarn Dec 6, 2023
Copy link
Contributor

@necolas necolas left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -6,24 +6,28 @@ This website is built using [Docusaurus 2](https://docusaurus.io/), a modern sta

- stylex actual package, and then the @stylexjs/babel-plugin plugin in node_modules

```bash
$ npm run build -w @stylexjs/stylex -w @stylexjs/shared -w @stylexjs/eslint-plugin -w @stylexjs/babel-plugin
Copy link
Contributor

Choose a reason for hiding this comment

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

npm run build --workspaces is all that's needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That builds all the packages/apps in the workspace, the documentation above only mentions the stylex packages and eslint plugin, so I only added builds that are required for docs to run (minimizes build time for first time contributors looking to only contribute to docs as size of monorepo increases). I could change it to build the entire workspace if that is the desired behavior 😀

Copy link
Contributor

@nmn nmn left a comment

Choose a reason for hiding this comment

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

Nice!

@nmn nmn merged commit 6263445 into facebook:main Dec 6, 2023
8 checks passed
@nmn
Copy link
Contributor

nmn commented Dec 6, 2023

Thanks for these great PRs @kevintyj!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants