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

Remove prepare script #1606

Closed
wants to merge 1 commit into from
Closed

Conversation

victorlin
Copy link
Member

@victorlin victorlin commented Dec 3, 2022

Description of proposed changes

The prepare script is automatically run with every call to npm ci / npm install, among other scripts¹. As it is just an alias to the build script (which can take several minutes), it should be safe to remove this automation and speed up the dev experience.

Updated README accordingly.

¹ https://docs.npmjs.com/cli/v8/using-npm/scripts?v=true#life-cycle-operation-order

Related issue(s)

N/A

Testing

@victorlin victorlin requested a review from a team December 3, 2022 03:10
@victorlin victorlin self-assigned this Dec 3, 2022
@nextstrain-bot nextstrain-bot temporarily deployed to auspice-victorlin-remov-lkcqdl December 3, 2022 03:10 Inactive
The prepare script is automatically run with every call to npm ci / npm
install, among other scripts¹. As it is just an alias to the build
script (which can take several minutes), it should be safe to remove
this automation and speed up the dev experience.

Some parts of the README and CI workflow had relied on the prepare
script automation. Updated accordingly.

¹ https://docs.npmjs.com/cli/v8/using-npm/scripts?v=true#life-cycle-operation-order
@victorlin victorlin force-pushed the victorlin/remove-prepare-script branch from dc99c86 to 35453f9 Compare December 3, 2022 04:14
@nextstrain-bot nextstrain-bot temporarily deployed to auspice-victorlin-remov-lkcqdl December 3, 2022 04:14 Inactive
@victorlin victorlin mentioned this pull request Dec 5, 2022
7 tasks
@victorlin
Copy link
Member Author

Closing this since the prepare script is still needed per discussion.

A more reasonable approach would be to document the --ignore-scripts option for npm ci, npm install, etc. for developers who wish to avoid the extra churn.

@victorlin victorlin closed this Dec 7, 2022
@victorlin victorlin deleted the victorlin/remove-prepare-script branch December 7, 2022 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

3 participants