Re: Ivy - PR-57 need inputs

2017-07-28 Thread Stefan Bodewig
On 2017-07-28, Jaikiran Pai wrote: > Ivy has a DependencyResolver interface which is the central piece of > contract/interface for extending Ivy (any external usage for that > matter). I'm not at all familiar with Ivy and the eco system that might exist around it. How likely is it that anybody

[GitHub] ant-ivy issue #57: fix last inconsistencies in generics

2017-07-28 Thread jaikiran
Github user jaikiran commented on the issue: https://github.com/apache/ant-ivy/pull/57 Overall, this looks fine to me. I just need some inputs from the rest of the team on one specific change, for which I have raised a dev list thread

Ivy - PR-57 need inputs

2017-07-28 Thread Jaikiran Pai
This PR - https://github.com/apache/ant-ivy/pull/57 does changes related to generics usage. I reviewed it a while back and it looks fine overall except for one change, for which I need inputs from the rest of the team. Ivy has a DependencyResolver interface which is the central piece of

[GitHub] ant-ivy issue #57: fix last inconsistencies in generics

2017-07-28 Thread twogee
Github user twogee commented on the issue: https://github.com/apache/ant-ivy/pull/57 The new method is taken from `AbstractOSGiResolver` where a corresponding method is made public. All resolvers implement only the new method and the fallback for the deprecated method is moved to

Ivy - PR-57 need inputs

2017-07-28 Thread Jaikiran Pai
This PR - https://github.com/apache/ant-ivy/pull/57 does changes related to generics usage. I reviewed it a while back and it looks fine overall except for one change, for which I need inputs from the rest of the team. Ivy has a DependencyResolver interface which is the central piece of

Re: Ivy - PR-57 need inputs

2017-07-28 Thread Gintautas Grigelionis
Please see my comment on GitHub. It is a change of signature (from array to collection) in order to change the return type (from array to collection, because arrays of generics do not work), and it's based on refactoring of AbstractOSGiResolver. It is a crucial piece to getting the generics right,

[GitHub] ant-ivy issue #57: fix last inconsistencies in generics

2017-07-28 Thread twogee
Github user twogee commented on the issue: https://github.com/apache/ant-ivy/pull/57 BTW, the old method is still there, the implementation is in the abstract class, `AbstractResolver`. --- If your project is set up for it, you can reply to this email and have your reply appear on

Re: Is CacheResolver relevant?

2017-07-28 Thread Gintautas Grigelionis
Here's a relevant discussion [1] with Xavier himself explaining the reason for cache resolver being there. [1] http://apache-ivy.996301.n3.nabble.com/Does-Ivy-really-have-a-cache-resolver-td3648.html Gintas P.S. It's unrelated, but, while researching, I came across this issue: [1]

Re: Ivy - PR-57 need inputs

2017-07-28 Thread Gintautas Grigelionis
Here's a japicmp report on binary incompatibilities between Ivy 2.4.0 and the snapshot build from the PR branch (which is rebased on latest master): none of them break binary compatibility. Would anybody comment the rest of incompatibilities? I am guilty of breaking ModuleRules, but that was for

Re: Ivy - PR-57 need inputs

2017-07-28 Thread Gintautas Grigelionis
Sorry all, I misunderstood the problem, and thanks to Jaikiran and Stefan for the discussion. Third party implementations will be broken because they lack the method with the new signature unless they extend the AbstractResolver. And the AbstractResolver must work the other way around, provide a

[GitHub] ant-ivy issue #57: fix last inconsistencies in generics

2017-07-28 Thread twogee
Github user twogee commented on the issue: https://github.com/apache/ant-ivy/pull/57 The same applies to `ChainResolver` (which should check resolver type as suggested by @jaikiran); actually, all custom resolvers that do not extend `AbstractResolver` could be wrapped in chain

[GitHub] ant-ivy issue #57: fix last inconsistencies in generics

2017-07-28 Thread twogee
Github user twogee commented on the issue: https://github.com/apache/ant-ivy/pull/57 The methods in `AbstractResolver` must be defined the other way around: the default implementation of the method with new signature should call the method with the old signature. May I amend the

Re: Ivy - PR-57 need inputs

2017-07-28 Thread Gintautas Grigelionis
It's not a new method, it's a change of signature. The old method is still there. If an implementation disregards the abstract class, which contains the replacement, it would still work, right? Gintas 2017-07-28 18:55 GMT+02:00 Stefan Bodewig : > On 2017-07-28, Jaikiran Pai

[GitHub] ant-ivy issue #57: fix last inconsistencies in generics

2017-07-28 Thread twogee
Github user twogee commented on the issue: https://github.com/apache/ant-ivy/pull/57 Is this sufficient, or should I add an extra check in `ChainResolver` ``` if (resolver instanceof AbstractResolver || hasGenericMethod(resolver)) { ... }