-
Notifications
You must be signed in to change notification settings - Fork 216
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
build: remove the build task from postinstall #5016
base: main
Are you sure you want to change the base?
build: remove the build task from postinstall #5016
Conversation
|
Branch previewReview the following VRT differencesWhen a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:
If the changes are expected, update the |
Tachometer resultsCurrently, no packages are changed by this PR... |
2d5df79
to
c3a109f
Compare
Pull Request Test Coverage Report for Build 13970585113Details
💛 - Coveralls |
469bcc7
to
1390494
Compare
9881a55
to
6849927
Compare
75da4af
to
647530c
Compare
fd0819b
to
6ad0ec1
Compare
de64457
to
d1d2d61
Compare
@@ -50,13 +50,14 @@ | |||
"new-package": "cd projects/templates && plop", | |||
"postcustom-element-json": "node ./tasks/run-check-cem.js", | |||
"postdocs:analyze": "node ./scripts/add-custom-properties.js --src=\"projects/documentation/custom-elements.json\"", | |||
"postinstall": "husky && patch-package && yarn get-ready", | |||
"postinstall": "husky || true && patch-package", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The husky updates here align with the documentation: https://typicode.github.io/husky/how-to.html
Yarn does not support prepare
commands.
@castastrophe can you please take a look at these failing tests |
d1d2d61
to
5601fd3
Compare
@TarunAdobe I already did and they appear connected to the typical flakiness of CI. I didn't see anything related to what this PR is updating. |
Description
This update removes the build from the postinstall task and updates documentation to ensure users are running the build after install manually. This also updates CI to ensure the build is completed during validations.
Motivation and context
Postinstall scripts make installs slower and riskier; additionally, postintall scripts do not report their output to the console out-of-the-box so any failures would happen quietly and be difficult to debug.
How has this been tested?
packages/accordion/src/Accordion.ts
but notpackages/accordion/src/Accordion.js
.git clean -dfX
yarn install
packages/accordion/src/Accordion.js
.Types of changes
Checklist
Best practices
This repository uses conventional commit syntax for each commit message; note that the GitHub UI does not use this by default so be cautious when accepting suggested changes. Avoid the "Update branch" button on the pull request and opt instead for rebasing your branch against
main
.