add warning from issue #2495 #2577
Conversation
src/reporters/lang/en.js
Outdated
| refusingDownloadGitWithoutCommit: 'Refusing to download the git repo $0 over plain git without a commit hash', | ||
| refusingDownloadHTTPWithoutCommit: 'Refusing to download the git repo $0 over HTTP without a commit hash', | ||
| refusingDownloadHTTPSWithoutCommit: 'Refusing to download the git repo $0 over HTTPS without a commit hash - possible certificate error?', | ||
|
|
||
There was a problem hiding this comment.
Could you please revert all the unnecessary changes, such as these spaces? In general, the Facebook coding style (which is somewhat used in the Yarn codebase too) is to trim all trailing whitespace from every line, so blank lines like these should be totally blank.
src/reporters/lang/en.js
Outdated
| @@ -257,6 +256,7 @@ const messages = { | |||
| optionalDepNotInstalled: 'Optional dependency $0 not installed', | |||
| packageWrongVersion: '$0 is wrong version: expected $1, got $2', | |||
| packageDontSatisfy: '$0 doesn\'t satisfy found match of $1', | |||
| packageContainsYarnAsGlobal: 'installing Yarn via Yarn will result in you having two separate versions of Yarn installed at the same time.', | |||
There was a problem hiding this comment.
Maybe also mention that it's not recommended, and to upgrade Yarn by following the instructions at https://yarnpkg.com/en/docs/install?
There was a problem hiding this comment.
OK so the full text will be:
installing Yarn via Yarn will result in you having two separate versions of Yarn installed at the same time, which is not recommended. To update Yarn please follow https://yarnpkg.com/en/docs/install
|
Hi @Daniel15 thank you for your review I've added the spaces and changed the message. |
src/cli/commands/global.js
Outdated
| @@ -178,7 +178,11 @@ const {run, setFlags: _setFlags} = buildSubCommands('global', { | |||
| await updateCwd(config); | |||
|
|
|||
| const updateBins = await initUpdateBins(config, reporter, flags); | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
nit: a few whitespaces got into this line
src/cli/commands/global.js
Outdated
|
|
||
| if (args.includes('yarn')) { | ||
| reporter.warn(reporter.lang('packageContainsYarnAsGlobal')); | ||
|
|
There was a problem hiding this comment.
nit: this empty line not needed here
There was a problem hiding this comment.
Thank you @bestander , I removed the empty line
There was a problem hiding this comment.
Sorry did it on my master now it's in
|
|
||
| if (args.includes('yarn')) { | ||
| reporter.warn(reporter.lang('packageContainsYarnAsGlobal')); | ||
|
|
|
Just one more space is unnecessary and could you rebase please? |
mnsn
force-pushed
the
issue-2495
branch
5 times, most recently
from
47c393b
to
1130b3d
Compare
|
Fixed in #2926 |
Summary
as request in issue #2495 we should warn user when installing yarn globally using yarn, I have added a warning massage in the right place
Test plan
no test required