-
-
Notifications
You must be signed in to change notification settings - Fork 76
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 --mutex network
argument for Yarn builds
#625
Changes from 4 commits
8866e7b
7556d86
b7b5e4e
b73c8ac
8afa526
e076190
a630140
50f36bc
738c258
5e9ca32
612216a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -90,6 +90,10 @@ | |
<node.version>you-must-override-the-node.version-property</node.version> | ||
<npm.version>you-must-override-the-npm.version-property</npm.version> | ||
<yarn.version>you-must-override-the-yarn.version-property</yarn.version> | ||
<!-- can be set to the empty string or install for yarn2 --> | ||
<!-- ensure only one concurrent 'yarn install' --> | ||
<!-- when yarn cache is empty, multiple yarns performing network fetches frequently results in opaque errors --> | ||
<yarn.args>---mutex network</yarn.args> | ||
scherler marked this conversation as resolved.
Show resolved
Hide resolved
|
||
<frontend-version>1.12.1</frontend-version> | ||
<nodeDownloadRoot>https://repo.jenkins-ci.org/nodejs-dist/</nodeDownloadRoot> | ||
<npmDownloadRoot>https://repo.jenkins-ci.org/npm-dist/</npmDownloadRoot> | ||
|
@@ -1326,9 +1330,7 @@ | |
</goals> | ||
<phase>initialize</phase> | ||
<configuration> | ||
<!-- ensure only one concurrent 'yarn install' --> | ||
<!-- when yarn cache is empty, multiple yarns performing network fetches frequently results in opaque errors --> | ||
<arguments>--mutex network</arguments> | ||
<arguments>${yarn.args}</arguments> | ||
jtnord marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok @basil , the only thing is that we do
So you are suggesting that would now become
I am fine with it, but personally would find it more readable, if we would drop the
wdyt? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should be clearer now my previously unpublished comments are published, the question remains is what (if anything) should the default be (keep the default as |
||
</configuration> | ||
</execution> | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
given we now require you to specify the yarn version (the previous release) I think we should just remove the property and the use of it on line 1333.
Then we document that users of yarn1 will need to add a property
frontend.yarn.arguments
to---mutex network
@basil removed the node/yarn properties so I think this may well be better suited.
if not and we want to keep the default the property needs to be changed to
<frontend.yarn.arguments>---mutex network</frontend.yarn.arguments> <!-- extra arguments needed for yarn -->
to match the property of the plugin to make the understanding and override of it easier.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yarnpkg/yarn#2146 (comment) you can as well create a
.yarnrc
and add--install.mutex network
in case you want it.