docs: point setup at .nvmrc and document how to run the tests - #1378
Open
ishan-one8 wants to merge 1 commit into
Open
ishan-one8 wants to merge 1 commit into
ishan-one8 wants to merge 1 commit into
Conversation
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Point setup at
.nvmrcand document how to run the testsWho this is for: someone cloning EmbeddedChat for the first time and following the README top to bottom. Right now that person is told to install Node 22, and
yarnthen refuses to install anything.Fixes #1377
Acceptance Criteria fulfillment
.nvmrcas the source of truth instead of a hard-coded major, and shownvm install/nvm usewith no argument, which read itpreinstallcheck is documented, including the error it prints and how to recover from itdevelopThe moment this documents
README.mdsays:while
.nvmrcpinsv24.0.0andscripts/node-check.jsenforces that major onpreinstall. Following the README exactly is what breaks the install, which is a bad first ten minutes for a new contributor.The second half is the thing I lost the most time to after that: nothing in the repo says how to run tests, and there is no
CONTRIBUTING.md. Finding out thatpackages/layout_editoruses Node's built-in runner, and that a package's tests must be run from inside that package — otherwise Babel resolves its root config from the wrong cwd and never findspackages/react/babel.config.js— is currently rediscovered by readingpackage.jsonfiles one by one.Video/Screenshots
Not applicable — documentation only.
PR Test Details
Each claim in the new text was checked against
developbefore writing it:cd packages/layout_editor && yarn test→ passes (node --test src/lib/*.test.js)packages/e2e-reacttestscript →playwright testpackages/reacthas notestscript, and running its existingsrc/index.test.jsthrough jest fails with "Cannot use import statement outside a module" — so the README now points at fix(react): src/index.test.js fails on develop due to missing swiper/element/bundle module #1191 rather than claiming it worksnvm install/nvm usewith no argument read.nvmrcNo code changes, so nothing to build or lint beyond the docs themselves.