[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-198910133 @dinoboy197: Sorry, I haven't had time to look at @ChristianSchulte suggestion until now. @jvanzyl @michael-o: Suggestion: Merge this solution. I'll evaluate the

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user dinoboy197 commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-198031554 @michael-o @jvanzyl @barthel - just checking in to see if any progress has been made on reviewing this. --- If your project is set up for it, you can reply to this

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user ChristianSchulte commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-188009400 @barthel: See [c31be833a6f8d9266990a708efe624d09fa06dec](https://github.com/ChristianSchulte/maven/commit/c31be833a6f8d9266990a708efe624d09fa06dec) --- If your

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-187570913 @ChristianSchulte : Sounds interesting. I'll take a look on it at the end of this week. --- If your project is set up for it, you can reply to this email and have your

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user michael-o commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-185895693 @barthel much but I won't be able to take a closer at it before next week. @jvanzyl if you can do it sooner, just go again and do it. --- If your project is set up for

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-185894318 @dinoboy197 : Fantastic. Thanks for your help. After reading your post, it sounds so logical. @michael-o , @jvanzyl : Now the merge comes true? :-D --- If your

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user dinoboy197 commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-185839435 @barthel - I think all you should need to do is to rebase your feature branch (MNG-3092) against upstream/master. Something like this should do it (assuming

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-184374596 @michael-o Sorry I've no idea how to remove the merge 'noise' from this really old branch. The only change of mine is ```28fd43d```. --- If your project is set up for

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user michael-o commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-184350865 This commit is huge and has a lot of merge noise. Do you think you can squeeze it down to your changes only? --- If your project is set up for it, you can reply to

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-184328357 @jvanzyl @michael-o : Read to merge. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-184327161 Integration test pull request: https://github.com/apache/maven-integration-testing/pull/14 --- If your project is set up for it, you can reply to this email and have

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-184308767 Use JSR 330 annotation instead of the Plexus annotation. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well.

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-184304203 @jvanzyl : Thanks for your tip. Now it works. :-D --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user jvanzyl commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-183894970 I suggest looking at more recent integration tests which are similar to yours where there is a default implementation and new implementations can be used. Take a look at

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-183661203 @jvanzyl , @michael-o : I start working on the integration tests. It's more complicated than I thought. The default handling works but I have difficulties to

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user jvanzyl commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-180506499 Not critical, I can add the annotations later. More important are integration tests if you haven't started those yet. If you have, great! --- If your project is set up

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-180504355 It's an old branch. So I merged the actual master into it and use Maven 3.4.0-SNAPSHOT instead. Only the last commit reflects the MNG-3092 solution.

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-178390879 @jvanzyl , @michael-o : Sorry for the delay. I'm busy right now. I will try to do the change to Maven 3.4 (and JSR330) and add a integration test. But not before friday

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user michael-o commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-174656884 Can we make any progress here? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user michael-o commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-174671286 This is what I have expected you to say ;-) @barthel or anyone else, can you craft up an IT for that? --- If your project is set up for it, you can reply to this

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user michael-o commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-174666838 @jvanzyl There are unit tests in the commit. Isn't that enough? Otherwise, I'd ask @barthel for it. --- If your project is set up for it, you can reply to this email

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user jvanzyl commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-174668942 For a feature like that there should be an integration test as well. --- If your project is set up for it, you can reply to this email and have your reply appear on

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user pSub commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-174664718 I would appreciate a merge, too. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user jvanzyl commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-174664828 If there are integration tests don't wait for me. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user jvanzyl commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-160451976 We're looking at the new year for the next core release so you have time. I might have my JSR330 changes done by then. I have 15 super nasty uses of Plexus to get around

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-160448962 I'm busy the next few days. But I will give it a try if these changes have a chance to merge in 3.3.10. --- If your project is set up for it, you can reply to this email

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user jvanzyl commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-159774014 @barthel I mean using the JSR330 equivalents for @Component and @Requirement. For @Component we replace them with @Named and @Singleton used together at the class level,

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user jvanzyl commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-159774308 @barthel I ask this to be done because with some recent changes in Sisu I actually have a chance to land my JSR330 branch which entirely removes Plexus from Maven.

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-159728334 @michael-o Indent size and inside issues solved. Commits squashed and amend on last commit in branch. What can I do to avoid the merge commit? If I merge the master

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-159729295 @jvanzyl Did you mean ```@Requirement``` and ```@Component```? I'm following the coding style found in the classes around. Please point me to the replacements

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user michael-o commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-159743197 @barthel While I am not a Git expert. I extract a patch/stash fo the work. Branch master, apply stash and commit again.

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user michael-o commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-159546743 There are still some issues with the branch. You did not replay on top of master. It is creating a merge commit which I'd like to avoid. Some other issues will be

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user jvanzyl commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-159585971 Just looking through here and you also switch these to use JSR330 annotations instead of the plexus annotations. Any additions I'd like to see use JSR330 which is what we

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-159399815 Organize import in ```DefaultVersionRangeResolver``` and squash commit. --- If your project is set up for it, you can reply to this email and have your reply appear on

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user michael-o commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-159366801 @barthel Git gives me: > git merge --no-commit barthel/MNG-3092 Auto-merging

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user michael-o commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-159045732 @barthel GitHub tells me that this branch has conflicts. Can you kindly check if rebase on top of master if necessary? --- If your project is set up for it, you can

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user michael-o commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-159078734 Thanks, I will test and merge tomorrow. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-159011269 I would like to see this changes in the next Maven (3.3.10) release. @michael-o @jvanzyl Please don't forget to merge :-) --- If your project is set up for it, you

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-159059568 @michael-o see my comment Oct 19: > Please remove version property for commons-lang when merging into the master. That's the rework you suggest with working

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user michael-o commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-159062461 @barthel See my inline comments. Then I will take it from there. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-159076106 POM file adapted. Use parent version 3.3.10-SNAPSHOT and remove ```commons-lang``` dependency. --- If your project is set up for it, you can reply to this email and

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user michael-o commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-151415114 @jvanzyl Are you saying that you dislike to change the default behavior, i.e., no merge? @barthel Did you run the ITs before and after your change? --- If your

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-151395866 Can I do anything to support the merge? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project

Re: [GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Changing the default behaviour of anything resolution related mid 3.x line is likely not a good idea. We definitely need to run the ITs with any changes like these to see what happens. Changing the way resolution works is definitely a 4.x thing. > On Oct 27, 2015, at 1:42 AM, michael-o

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-151463857 The default behavior is ``do nothing``. The ITs doesn't show any errors at this modifications. --- If your project is set up for it, you can reply to this email and have

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user jvanzyl commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-151499565 Then I don’t see an issue with merging it. > On Oct 27, 2015, at 4:38 AM, barthel wrote: > > The default behavior is do

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-150489457 @michael-o @jvanzyl Let's go merging? :-) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user michael-o commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-149485116 @jvanzyl Can a I merge this PR into master? Are you OK with that? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user michael-o commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-149484847 @barthel Thanks for the update. If you rebase against current master you don't need that property at all. I have added Commons Lang to dependency management already.

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user michael-o commented on a diff in the pull request: https://github.com/apache/maven/pull/70#discussion_r42421348 --- Diff: maven-aether-provider/src/main/java/org/apache/maven/repository/internal/DefaultVersionRangeResolver.java --- @@ -152,6 +154,16 @@ public

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user michael-o commented on a diff in the pull request: https://github.com/apache/maven/pull/70#discussion_r42421297 --- Diff: .gitignore --- @@ -13,3 +13,5 @@ out/ /bootstrap /dependencies.xml .java-version +nb-configuration.xml + --- End diff --

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-149349259 @michael-o Your review comments are integrated. .gitignore cleaned up. Apache Commons Lang used instead of ``IllegalArgumentException``. Please remove version

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-148898986 This solution could also be usable for [MNG-5353](https://issues.apache.org/jira/browse/MNG-5353). --- If your project is set up for it, you can reply to this email and

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-148653328 @michael-o There is a typo and not a hidden statement. :-D --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well.

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user michael-o commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-148634870 @ChristianSchulte Maybe `Rage` is intentional ;-) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-148659197 @michael-o Your comment on 'old' commit b6d72699c8b0089351f797e217d635424c1ae5a2: > Don't! Always through a NPE if null is passed. :+1: But, I'm oriented on

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-148704373 @ChristianSchulte Typo fixed. Thanks for looking into it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well.

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-148553852 This PR replaced the PR (#68) and based on master and re-created branch MNG-3092. This branch contains the cherry picked b6d7269. --- If your project is set up for it,

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

GitHub user barthel opened a pull request: https://github.com/apache/maven/pull/70 MNG-3092: Adds version range result filter behaviour The discussion on issue [MNG-3092](https://issues.apache.org/jira/browse/MNG-3092) shows the seriously needs of different kinds of version range

[GitHub] maven pull request: MNG-3092: Adds version range result filter beh...

Github user barthel commented on the pull request: https://github.com/apache/maven/pull/70#issuecomment-148556469 @jvanzyl @michael-o : Ready for easier review. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your