Skip to content
This repository has been archived by the owner on Sep 16, 2024. It is now read-only.

remove flag from version filter #1

Merged
merged 1 commit into from
Jun 1, 2021
Merged

remove flag from version filter #1

merged 1 commit into from
Jun 1, 2021

Conversation

vvcephei
Copy link
Member

@vvcephei vvcephei commented Jun 1, 2021

Remove the default-disabled feature flag from the version-filter patch.

The flag was added in case the patch didn't work as expected, but
in that case, users can always just fall back to a vanilla release of
Maven.

I think there are two good reasons to just enable the patch unconditionally:

  1. As is, having this flag makes it difficult to use, since you always
    have to remember the flag, and if you forget, you're in for a couple of hours
    of downloading pom files.
  2. Just patching the binary lets us swap out the Maven binary in IDEA as well,
    which means we get the benefit of this patch while resolving dependencies
    in the IDE. In contrast, there's no way to supply a flag to be used with the
    maven wrapper in a way that all IDE operations avoid downloading the
    unnecessary older versions.

@vvcephei
Copy link
Member Author

vvcephei commented Jun 1, 2021

Hey @xli1996 , what do you think about removing the feature flag? My reasoning is above.

If you want to accept this patch, I think we'll also have to bump the versions in this repo, but I wanted to run it by you first.

@xli1996 xli1996 merged commit 831c096 into confluentinc:maven-3-8-1-patch Jun 1, 2021
@vvcephei
Copy link
Member Author

vvcephei commented Jun 1, 2021

To test this patch:

I built it:

 mvn -DdistributionTargetDir="$HOME/maven-custom/" clean package

Then, I cleaned up my env:

cd ksql
mvn clean
rm -r ~/.m2/repository
~/maven-custom/bin/mvn clean compile

I visually verified that only the latest versions get resolved, and also the build takes about 12 minutes, while it would otherwise take hours.

I also tested this in IDEA by setting Settings > Build,Execution,Deployment > Build Tools > Maven > Maven home path to /home/john/maven-custom. Then, I deleted the ksql project, cleared caches, did another mvn clean and another rm -r ~/.m2/repository. Upon importing the ksql project again, I observed that it does have to resolve all artifacts, but it only resolves the latest ones, and again the whole import takes about 12 minutes.

@vvcephei vvcephei deleted the john-maven-patch-patch branch June 1, 2021 21:21
vvcephei added a commit to confluentinc/ksql that referenced this pull request Jun 2, 2021
…ion (#7620)

* Update the wrapper to include confluentinc/maven#1
* Document what the wrapper does, and how to use it on the CLI and in IDEs

Reviewers: Leah Thomas
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants