CASSANDRA-16597 - introduce Maven wrapper to the build process for Maven-less environments #963
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
smiklosovic
force-pushed
the
CASSANDRA-16597
branch
from
676f855
to
936f783
Compare
michaelsembwever
approved these changes
Apr 14, 2021
smiklosovic
force-pushed
the
CASSANDRA-16597
branch
from
936f783
to
ddac95d
Compare
smiklosovic
force-pushed
the
CASSANDRA-16597
branch
2 times, most recently
from
eef53eb
to
c3593d9
Compare
…ven-less build environments
smiklosovic
force-pushed
the
CASSANDRA-16597
branch
from
c3593d9
to
bc5f1cd
Compare
dcapwell
reviewed
May 11, 2021
Comment on lines
+219
to
+222
| while IFS="=" read key value; do | ||
| case "$key" in (wrapperUrl) jarUrl="$value"; break ;; | ||
| esac | ||
| done < "$BASE_DIR/.mvn/wrapper/maven-wrapper.properties" |
There was a problem hiding this comment.
You could source the file; its bash compatible
source "$BASE_DIR/.mvn/wrapper/maven-wrapper.properties"
echo "$jarUrl"
There was a problem hiding this comment.
bear in mind this is just a copy of an upstream script: https://github.com/takari/maven-wrapper/blob/master/mvnw
There was a problem hiding this comment.
yeah Mick is right, this is what upstream contains ... I do not think we are going to mess with that and introduce some custom stuff into it.
Comment on lines
+231
to
+250
| if command -v wget > /dev/null; then | ||
| if [ "$MVNW_VERBOSE" = true ]; then | ||
| echo "Found wget ... using wget" | ||
| fi | ||
| if [ -z "$MVNW_USERNAME" ] || [ -z "$MVNW_PASSWORD" ]; then | ||
| wget "$jarUrl" -O "$wrapperJarPath" | ||
| else | ||
| wget --http-user=$MVNW_USERNAME --http-password=$MVNW_PASSWORD "$jarUrl" -O "$wrapperJarPath" | ||
| fi | ||
| elif command -v curl > /dev/null; then | ||
| if [ "$MVNW_VERBOSE" = true ]; then | ||
| echo "Found curl ... using curl" | ||
| fi | ||
| if [ -z "$MVNW_USERNAME" ] || [ -z "$MVNW_PASSWORD" ]; then | ||
| curl -o "$wrapperJarPath" "$jarUrl" -f | ||
| else | ||
| curl --user $MVNW_USERNAME:$MVNW_PASSWORD -o "$wrapperJarPath" "$jarUrl" -f | ||
| fi | ||
|
|
||
| else |
There was a problem hiding this comment.
can we not just use curl? curl is on most systems (wget is not), so we could simplify this to just be the curl command. By doing this we can also drop the MavenWrapperDownloader class all together (less to support)
| if [ -z "$MVNW_USERNAME" ] || [ -z "$MVNW_PASSWORD" ]; then | ||
| curl -o "$wrapperJarPath" "$jarUrl" -f | ||
| else | ||
| curl --user $MVNW_USERNAME:$MVNW_PASSWORD -o "$wrapperJarPath" "$jarUrl" -f |
slachiewicz
reviewed
May 11, 2021
|
Still feeling that all that mix of Ant/Maven configurations should be in long-term simplified - with pure Maven setup. |
smiklosovic
force-pushed
the
CASSANDRA-16597
branch
from
5b858d3
to
48b8f1f
Compare
adelapena
pushed a commit
to adelapena/cassandra
that referenced
this pull request
Feb 12, 2024
Speculative reads will now be triggered after sampleReadLatency past the replica reads are issued, instead of past queryStartNanoTime
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.