Have apply-error-prone-suggestions.sh download JitPack-hosted artifacts
#441
Conversation
|
Looks good. No mutations were possible for these changes. |
There was a problem hiding this comment.
So as indicated this is not a perfect fix, but it's the easiest thing I can do on short notice. (Modifying pom.xml isn't harder, but then there are more cases I'd have to reason about, including any possible impact on the next release to Maven Central.) As this is an improvement regardless, I think it's an acceptable change.
CC @benhalasi in case you want to try it :)
| @@ -9,13 +9,14 @@ | |||
| set -e -u -o pipefail | |||
|
|
|||
| if [ "${#}" -gt 1 ]; then | |||
| echo "Usage: ./$(basename "${0}") [PatchChecks]" | |||
| echo "Usage: ${0} [PatchChecks]" | |||
There was a problem hiding this comment.
We no longer assume that the script is executed from the directory in which it resides.
| exit 1 | ||
| fi | ||
|
|
||
| patchChecks=${1:-} | ||
|
|
||
| mvn clean test-compile fmt:format \ | ||
| -s "$(dirname "${0}")/settings.xml" \ |
There was a problem hiding this comment.
This fix has downsides of its own: it overrides the user's settings.xml, which may contain other useful or even required settings. For Picnic devs it means that our Nexus deployment is bypassed, triggering one extra round of downloads.
The alternative is to move the settings.xml contents into a profile in the top-level pom.xml, but I seem to recall that Maven Central doesn't like that. (Or perhaps is does allow it when part of a profile? I'd have to check.)
There was a problem hiding this comment.
Based on Setting up multiple repositories (esp. the order) and Using mirrors for repositories; as Picnic devs mirror everything anyway, could we instead just place repositories in the root of the pom (i.e. not in a profile)? Then we can also omit the settings.xml altogether.
To my understanding, this would behave the same as that the super pom defines a repository to Maven central, which is always included too.
rickie
added
the
chore
There was a problem hiding this comment.
Thanks for the quick fix @Stephan202 !
Would indeed be nice if @benhalasi is up for checking if with these changes it'd work 😄.
Verified that it still works for me :).
rickie
force-pushed
the
sschroevers/use-custom-maven-settings
branch
from
3922e27
to
82a929f
Compare
|
Looks good. No mutations were possible for these changes. |
rickie
force-pushed
the
sschroevers/use-custom-maven-settings
branch
from
82a929f
to
11cc582
Compare
|
Looks good. No mutations were possible for these changes. |
| exit 1 | ||
| fi | ||
|
|
||
| patchChecks=${1:-} | ||
|
|
||
| mvn clean test-compile fmt:format \ | ||
| -s "$(dirname "${0}")/settings.xml" \ |
There was a problem hiding this comment.
Based on Setting up multiple repositories (esp. the order) and Using mirrors for repositories; as Picnic devs mirror everything anyway, could we instead just place repositories in the root of the pom (i.e. not in a profile)? Then we can also omit the settings.xml altogether.
To my understanding, this would behave the same as that the super pom defines a repository to Maven central, which is always included too.
…acts While there, tweak the usage message of both `apply-error-prone-suggestions.sh` and `run-mutation-tests.sh`.
rickie
force-pushed
the
sschroevers/use-custom-maven-settings
branch
from
11cc582
to
4c4794b
Compare
|
Looks good. No mutations were possible for these changes. |
|
I've checked and works well for me too. @rickie |
Thanks for verifying @benhalasi ! |
rickie
changed the title
apply-error-prone-suggestions.sh download Jitpack-hosted artifactsapply-error-prone-suggestions.sh download JitPack-hosted artifacts
Suggested commit message: