Repository navigation
[rush] Write the npm/Yarn "Lockfile has changed" warning to stderr - #6197
Open
PranavThukral wants to merge 1 commit into
Open
PranavThukral wants to merge 1 commit into
PranavThukral wants to merge 1 commit into
Conversation
When a non-pnpm lockfile is modified in the diff range, ProjectChangeAnalyzer printed "Lockfile has changed and lockfile content comparison is only supported for pnpm" with writeLine, which goes to stdout. That breaks `rush list --json --impacted-by git:<ref> | jq`. microsoft#5804 already moved the "created or deleted" message to stderr; this does the same for the modified case and adds tests for both.
Author
|
@microsoft-github-policy-service agree |
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.
Summary
In npm and Yarn repos,
rush list --json --impacted-by git:<ref>prints this line to stdout before the JSON whenever the committed lockfile was modified in the diff range:Because it lands on stdout, piping the output into
jqor a CI script fails to parse. This PR writes that message to stderr instead, so stdout stays valid JSON.Details
PR #5804 fixed the same issue for the similar "Lockfile was created or deleted" message in
ProjectChangeAnalyzer.getChangedProjectsAsync, but the npm/Yarnelsebranch a few lines below still usesterminal.writeLine, even though that branch is generally the more common one as lockfiles are modified on almost every dependency update.This change switches both forms of that message (subspace & non-subspace) from
writeLinetowriteWarningLine, matching #5804. As subspaces only work with pnpm, the copy never prints in practice. But, I changed for consistency.Nothing else was changed, only the output stream was affected. No other compatibility or performance impact beyond the message going to stderr.
How it was tested
M) and a created (A)npm-shrinkwrap.json, the latter checking [rush] Route lockfile-changed warning to stderr (fixes #5406) #5804. Each case shows that the message goes to warning output, not normal output.heft test --test-path-pattern ProjectChangeAnalyzer: 48 passed, no lint warnings.modifiedcase fails, so the test catches the bug.