-
Couldn't load subscription status.
- Fork 184
[MDEP-425] List plugin repositories via maven-artifact-transfer. #51
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
Conversation
| <properties> | ||
| <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> | ||
| </properties> | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could pluginRepository (I propose id of fake-remote-repository which is excused already in settings.xml) be defined in this IT and verified in test output?
* don't care about old versions * clean up some dead code
* static import
* spelling
This closes apache#47
…t pom in reactor build This closes apache#46
…ndencies for a specified artifact. (apache#57) * Created new mojo GetClassesMojo which lists all class dependencies for a specified artifact * adding test file and fixing small function name * Updating test functions names to be self-describing * Fixing styling issues preventing build completion * Removed return statements from test cases, removed unneeded piece of recycled code for building the ProjectBuildRequest * Changed GetClassesMojo to ListClassesMojo, fixed comments, changed transitive default value to false, changed ListClassesMojo methods to private if able, replaced assert in test case. * Changed modifying methods to private since outside classes do not interact with the mojo, fixed comments to follow oracle javadoc guidelines, changes method names to be more descriptive of their purpose, fixed shadowing variable names and renamed variables to be more descriptive * Removed unneccessary setter and getter methods, made the mojo in the test class a private field, included printing logic to use a try-with-resource statement to guarantee JarFile closure, reverted thrown error message when resolving dependencies to original to be less confusing.
| } | ||
|
|
||
| /** | ||
| * @return Returns a new ProjectBuildingRequest populated from the current session and the current project remote |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Returns --> returns
repositories, used --> Repositories. Used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented correct JavaDoc
| } | ||
| } | ||
|
|
||
| private void printClassesFromArtifactResult( ArtifactResult result ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete "FromArtifactResult" as its redundant with the argument
| // filter out files that do not end in .class | ||
| if ( !entryName.endsWith( ".class" ) ) | ||
| { | ||
| continue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider reversing the if and rewriting without continue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added via rebase of master. Should it be changed in this branch?
| { | ||
| coordinate.setType( tokens[3] ); | ||
| } | ||
| if ( tokens.length == 5 ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
= for safety
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added via rebase of master. Should it be changed in this branch?
| return buildingRequest; | ||
| } | ||
|
|
||
| protected ArtifactCoordinate toArtifactCoordinate( DependableCoordinate dependableCoordinate ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why protected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added via rebase of master. Should it be changed in this branch?
| * | ||
| * @param artifactString Coordinates of the Maven project to get. | ||
| * @return New Maven project. | ||
| * @throws MojoExecutionException If there was an error while getting the Maven project. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no period, no cap
| Artifact artifact = artifactResolver.resolveArtifact( pbr, coordinate ).getArtifact(); | ||
| return projectBuilder.build( artifact.getFile(), pbr ).getProject(); | ||
| } | ||
| catch ( Exception e ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
avoid catching Exception. Can this be more specific?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Catch thrown exceptions.
src/site/apt/usage.apt.vm
Outdated
|
|
||
| * <<<dependency:list-repositories>>> | ||
|
|
||
| This goal is used to list all the plugin repositories that this build depends upon. It will show plugin repositories defined in your settings, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is used to list --> lists
will show --> shows
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented correct English...
src/site/apt/usage.apt.vm
Outdated
| mvn dependency:list-plugin-repositories | ||
| +-----+ | ||
|
|
||
| Optionally, in verbose or debug mode it will display the location of the listed plugin repository: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will display --> displays
(Tech writing lives in the eternal present.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed.
| ConcreteDependencyMojo cdm = new ConcreteDependencyMojo(); | ||
| cdm.session = mavenSession; | ||
|
|
||
| Field par = AbstractDependencyMojo.class.getDeclaredField( "remoteRepositories" ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is "par"? perhaps a more descriptive name?
|
@pmoerenhout and other thanks for efforts ... we should not use maven-artifact-transfer at all So I close it now. |
|
Resolve #868 |
Updated the MDEP-425 with the use of the 0.13.0 version of maven-artifact-transfer.