Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SHRINKWRAP-261] Maven shortcut API #6

Closed
wants to merge 8 commits into from

Conversation

samaxes
Copy link
Contributor

@samaxes samaxes commented Sep 19, 2011

Shorthand for including maven artifacts
https://issues.jboss.org/browse/SHRINKWRAP-261

@samaxes
Copy link
Contributor Author

samaxes commented Sep 19, 2011

Andrew, there is a reason for this, from https://issues.jboss.org/browse/SHRINKWRAP-261:

The code uses the Validate class from impl-maven, so if no one has any objection to this, the Shortcut API will be on impl-maven project and not on api-maven.

Should we move Validate class to api-maven?

@kpiwko
Copy link
Member

kpiwko commented Sep 20, 2011

Samuel, why you recopied all the MavenBuilderImpl internals into MavenResolver? Any tough points to use it directly? I don't like the idea of having similar functionality twice in the code.

Andrew, indirection requires rewriting of DependencyBuilderInstantiator. It would make sense to have it done similarly to LoadableExtension which is in Arquillian. I think this might be done during API cleanup, where we should actually think of hiding all of impl classes, which might confuse user. I have no problem to accept in impl-maven and refactor later.

@samaxes
Copy link
Contributor Author

samaxes commented Sep 21, 2011

Karel, honestly I don't remember the reason anymore.
I've implemented it back in June - shrinkwrap/shrinkwrap#25.

I will have a closer look at this by the weekend.
If you have any suggestion please let me know.

@samaxes
Copy link
Contributor Author

samaxes commented Oct 18, 2011

Hi Karel and Andrew,

I'm pretty confident that this is very close to the best we can do with the current API.

Only a very compact class (Maven class) is exposed to the user under the Maven API.
Most of the implementation code is "hidden" in the impl-maven module.

The new Shortcut API can be used like this:

Maven.dependency("org.jboss.shrinkwrap.test:test-deps-c:1.0.0")
Maven.dependencies("org.jboss.shrinkwrap.test:test-deps-c:1.0.0", "org.jboss.shrinkwrap.test:test-deps-g:1.0.0")
Maven.withPom("test-pom.xml").dependency("org.jboss.shrinkwrap.test:test-deps-c:1.0.0")
Maven.withPom("test-pom.xml").dependencies("org.jboss.shrinkwrap.test:test-deps-c:1.0.0", "org.jboss.shrinkwrap.test:test-deps-g:1.0.0")

Collection<ARCHIVEVIEW> archiveViews = new MavenBuilderImpl(system, session, settings, dependencies, versionManagement)
.resolveAs(archiveView);

return (archiveViews == null || archiveViews.isEmpty()) ? null : archiveViews.iterator().next();
Copy link
Member

Choose a reason for hiding this comment

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

This condition is not enough, you should verify there was exactly one jar

@samaxes
Copy link
Contributor Author

samaxes commented Oct 20, 2011

All changes done, please validate.

@kpiwko
Copy link
Member

kpiwko commented Oct 23, 2011

Merged and pushed upstream as c2367b5. Thanks!

@kpiwko kpiwko closed this Oct 23, 2011
@samaxes samaxes deleted the SHRINKWRAP-261 branch May 15, 2013 23:27
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