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 redundant Auspice build #120

Closed
wants to merge 1 commit into from

Conversation

victorlin
Copy link
Member

@victorlin victorlin commented Dec 16, 2022

Description of proposed changes

Auspice has a prepare script that runs the build script automatically after npm install. This means the explicit call to run the build script is redundant and can be removed to improve build times.

Related issue(s)

Testing

  • Checks pass
  • nextstrain view --docker --image nextstrain/base:branch-victorlin-remove-redundant-auspice-build works

Auspice has a prepare script that runs the build script automatically
after npm install. This means the explicit call to run the build script
is redundant and can be removed to improve build times.
@victorlin
Copy link
Member Author

CI passes, but nextstrain view is unable to find '/nextstrain/auspice/dist' and fails.

I think this has to do with the upgrade to Node.js 16. Compare 1afd07d (which introduces the Node.js 16 upgrade) and the previous commit f8ea4da. Node.js 14 logs only show one occurrence of node auspice.js build --verbose while Node.js 16 logs show two (one being redundant).

Both NPM v7 (packaged with Node.js 14) and NPM v8 (packaged with Node.js 16) docs say that the prepare script should run on local npm install (and this is what I've observed on my local machine), so I'm not sure why there is a difference in behavior here. But given what's observed, this change should be bundled with the upgrade to Node.js 16.

@victorlin victorlin closed this Dec 20, 2022
@victorlin victorlin deleted the victorlin/remove-redundant-auspice-build branch December 20, 2022 20:51
@victorlin victorlin mentioned this pull request Dec 20, 2022
1 task
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.

1 participant