Skip to content

[rush-lib] Don't interrupt the installation process if the user hasn't enabled the inject dependencies feature. #4830

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

Merged
merged 3 commits into from
Jul 12, 2024

Conversation

L-Qun
Copy link
Contributor

@L-Qun L-Qun commented Jul 11, 2024

Summary

Yesterday, we received an on-call stating that a user experienced a failure in the CI installation process, even after executing rush update.

img_v3_02cm_acf52fae-7ae9-4f2c-9e6e-f5273b45c7hu

After investigating, we discovered that the repo-state.json file includes the packageJsonInjectedDependenciesHash, but alwaysInjectDependenciesFromOtherSubspaces is not enabled.

img_v3_02cm_4b8fd515-ab1f-4803-9dc1-22bc7bc19ehu

We believe this error negatively impacts the user experience. Users do not know how to resolve the issue after receiving the error message. So, we made a small modification to the source code:

I think a better way is to give a warning without interrupting the installation process.

How it was tested

  1. Remove alwaysInjectDependenciesFromOtherSubspaces from thepnpm-config.json file.
  2. Run rush update.
  3. Run rush install.
  4. Installion successful.

Impacted documentation

None.

@L-Qun L-Qun changed the title give a warning without interrupting the installation process [rush-lib] Don't interrupt the installation process if the user hasn't enabled the inject dependencies feature. Jul 11, 2024
@octogonz octogonz merged commit 598d622 into microsoft:main Jul 12, 2024
5 checks passed
@octogonz
Copy link
Collaborator

🚀 Finally released with Rush 5.129.7, thanks for your patience!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

4 participants