Skip to content

SHRINKRES-5 Added method to use remote repositories without using XML fi...#63

Closed
marcanpilami wants to merge 0 commit intoshrinkwrap:masterfrom
marcanpilami:master
Closed

SHRINKRES-5 Added method to use remote repositories without using XML fi...#63
marcanpilami wants to merge 0 commit intoshrinkwrap:masterfrom
marcanpilami:master

Conversation

@marcanpilami
Copy link
Copy Markdown
Contributor

Hello,

Looking for a simple Maven dependencies resolver/downloader, I eventualy came to the conclusion that SW resolver was the best in town but for one thing: it does not allow to add remote repositories through the API (it uses whatever repositories contained in the different maven conf files), which is a requirement of my current project (it is possible to interpret SHRINKRES-5 as the same issue, but I'm not sure). So I just forked it and added the function in a consistent way with the fluent API: [resolve()].withMavenRemoteRepo(name, url, layout). Test case is provided.

Please feel free to reuse it in case you deem it worthy of inclusion, and in any case thanks for your work on SW resolver.

Signed-off-by: marcanpilami marsu_pilami@msn.com

@kpiwko
Copy link
Copy Markdown
Member

kpiwko commented Feb 12, 2014

Hello,

I think this is very good start, thank you very much!

Currently, I have updated to latest Aether and this will unfortunately need a rebase. It would be great if we could mirror their RemoteReposity.Builder - http://download.eclipse.org/aether/aether-core/0.9.0.M2/apidocs/org/eclipse/aether/repository/RemoteRepository.Builder.html

The same way as we allow for dependencies, either string based coordinates or full MavenDependency object (built via MavenDependencies), we should follow the same approach. People will mostly use string based part anyway, but flexibility FTW.

Would do you think about builder approach? Would you find it handy in your project?

Thanks,

Karel

@marcanpilami
Copy link
Copy Markdown
Contributor Author

Actually, I am in the string users - my problem is that repositories come from a database as strings, while poms do not specify them (actually, most of time, the database contains the urls of Nexus proxies, since there is no internet access in most of my cases and therefore no access to the original repositories).

That being said, I agree that an API should be flexible, and see absolutely no issue in adding another method "withMavenRemoteRepo(RemoteRepository rr)". If you have a branch on GitHub containing your current work with the latest Aether, I can do the rebase myself and add that method if you like.

Marc-Antoine

@ALRubinger
Copy link
Copy Markdown
Member

+1 to the idea and thanks for your contributions!

My comments in the commit. As this involves public API enhancements you'll see we're a bit more careful. ;)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an interesting point. In further repository handling, we are not actually filtering for unique ids. I believe this step should be done at the end of the method and current handling of remote repos might be buggy as Maven requires ids to be unique.

Here, I imagine that you'd have an intermediate object and you'll want to change the repository with given id for single call - e.g. rewriting central to foo once, to bar second. So allowing specifying same id multiple times make sense to me.

@kpiwko
Copy link
Copy Markdown
Member

kpiwko commented Feb 13, 2014

Marc-Antoine, current master is the branch that contains Aether update. Adding RemoteRepository method would be awesome!

Definitely, I agree with the point that Strings will be most commonly used in reality to configure additional Maven Repositories.

@marcanpilami
Copy link
Copy Markdown
Contributor Author

Nice to see interest in this, thanks.
Modifications done (javadoc, args checks, repo builder, repo overload and all the related tests).

Now, doing a real rebase was a bad idea - it seems GitHub has lost track of the commits in the pull request. Anyway, it is in https://github.com/marcanpilami/resolver/tree/SW5. Sorry about that.

Waiting for your comments :-)

@kpiwko
Copy link
Copy Markdown
Member

kpiwko commented Feb 18, 2014

I'm closing this PR. Could you please create a new one using your SW5 branch? I think it would be much better for review and comments ;-)

@kpiwko kpiwko closed this Feb 18, 2014
@marcanpilami marcanpilami mentioned this pull request Feb 18, 2014
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants