Skip to content

Conversation

@pmoerenhout
Copy link
Contributor

Updated the MDEP-425 with the use of the 0.13.0 version of maven-artifact-transfer.

<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
</properties>

Copy link
Contributor

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?

elharo and others added 9 commits June 18, 2020 21:43
* don't care about old versions

* clean up some dead code
…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.
@elharo elharo changed the title MDEP-425: List plugin repositories via maven-artifact-transfer. [MDEP-425] List plugin repositories via maven-artifact-transfer. Jun 19, 2020
}

/**
* @return Returns a new ProjectBuildingRequest populated from the current session and the current project remote
Copy link
Contributor

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

Copy link
Contributor Author

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 )
Copy link
Contributor

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;
Copy link
Contributor

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

Copy link
Contributor Author

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 )
Copy link
Contributor

Choose a reason for hiding this comment

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

= for safety

Copy link
Contributor Author

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 )
Copy link
Contributor

Choose a reason for hiding this comment

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

why protected?

Copy link
Contributor Author

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.
Copy link
Contributor

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 )
Copy link
Contributor

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Catch thrown exceptions.


* <<<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,
Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implemented correct English...

mvn dependency:list-plugin-repositories
+-----+

Optionally, in verbose or debug mode it will display the location of the listed plugin repository:
Copy link
Contributor

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.)

Copy link
Contributor Author

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" );
Copy link
Contributor

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?

@slawekjaranowski
Copy link
Member

@pmoerenhout and other thanks for efforts ... we should not use maven-artifact-transfer at all
It should be reimplemented with using direct resolver API like I did in #301

So I close it now.

@jira-importer
Copy link

Resolve #868

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.

7 participants