Create a new pull request by comparing changes across two branches #104
Conversation
This file contains 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
We only need it for `scripts/publish.js`
This removes table output from `hook`, `profile`, `doctor`, `org`, and `token`. It also removes table output from the `--long` or `--dry-run` output of install commands. Table output is discouraged in a cli for accessibility reasons. The tests for `token` were also rewritten because they did not actually test the behavior of npm with the registry. `this.config` was also removed from `token`. npm-registry-fetch pulls all this from flatOptions already.
This removes table output from `npm search` and from all commands that log a summary of tarball content (`npm publish` and `npm pack`). Table output is discouraged in a cli for accessibility reasons.
This tiny single use package can live inline with where it's used
Eventually we'll use npm to do this so we don't have to check if color is enabled
It is only used during CI and we can call it with `npx` Co-authored-by: Gar <gar+gh@danger.computer>
Arborist CI has started failing on `macos-latest` now that those runners default to `arm64` machines (aka Apple Silicon). I am able to reproduce the failures locally on a Macbook Pro M1. After spending some time debugging the issue I believe it has to do with the timing of `Node` vs `Link` creation. I was able to bisect and find #5376 which removed the ability for nodes to possibly take longer to create than their link targets. Going back to the commit before that PR the flaky test passes locally for me and fails starting with the first commit in that PR. I'm just running the offending test in a loop and seeing if it fails, so not a perfect metric. But when it fails, I get a failure at least 10% of the time. On the old commit I was able to run it 50x with no failures. Here's what I was running locally to observe failures: ```sh COUNT="0" while true; do COUNT=$((COUNT+1)) echo "Start $COUNT" if ! npm test -w workspaces/arborist --ignore-scripts -- test/arborist/load-actual.js --no-coverage -Rtap --grep selflink; then echo "Failed on run $COUNT" exit 1 fi done ``` This is definitely an edge case, but one I would like to fix in the future. Disabling this test is to temporarily get CI green while we release and make more substantial changes that are hard to do with CI flaking. We've had other issues with symlinks and I would feel much better knowing we have defined behavior in this specific case when tracking down future potential symlink bugs. One fix that worked locally is iterating over `node.target.children` sequentially instead of in `Promise.all`] but that is probably only a side effect of the dep ordering in the test. A fix will have to account for any order of links and node taking different amount of time. https://github.com/npm/cli/blob/c1152e9d2e325bc87176d3d9788605107d109b7b/workspaces/arborist/lib/arborist/load-actual.js#L337
Removed all the setters, they were dangerous and not the right way to set those values. Tweaked `this.#init` so it always returns an object so the coerce in `this.load` didn't need to happen.
This is what npm changes it do during publish anyways
The previous change was made as part of the removal of the old progress bar for doctor. Log messages for each individual file scanned were moved from showing up in the progress message only to `log.silly`. There can be a very large amount of files scanned, and there is no reason to log each one individually.
Closes #7425 This is a little hard to test because everything should continue to work without progress, as evidenced by the lack of churn in the tests and snapshots here. The only existing tests that changed are the addition of newlines after prompts which is new behavior as part of this PR. I resorted to manually running some commands to get an idea of how the various states worked together: **`node . exec -- npm@4`** This should show progress at the start and then hide it as it prompts the user to install `npm@4`. `Ctrl+C` should exit from the prompt and the error should display on the next line (this is a current bug). **`node . audit signatures --loglevel=http`** This should show a lot of http log messages while always keeping the spinner on the last line of output. The spinner also should not jump between frames regardless of how quickly the log messages show up. **`node . pack`** This should immediately show the banners and output from `prepack` and not show the spinner until the actual packing is happening. **`node . login`** The spinner should never while the prompt is being displayed. **`node . view npm`** The spinner should appear while data is being fetched and then disappear for the rest of the command as output is being displayed. The end output on completion should not have any spinner frames rendered on new lines in the output. The old progress bar achieved this by calling `npmlog.disableProgress()` but now it is able to hide the spinner appropriately even while outputting individual lines to stdout.
GulajavaMinistudio
merged commit 61e2ecc
into
GulajavaMinistudio:latest
43 of 44 checks passed
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.
No description provided.