Skip to content

Add TestGenerateMojo to the akka-grpc-maven-plugin#1100

Merged
raboof merged 1 commit intoakka:masterfrom
samantmaharaj:maven-plugin-test-generate
Aug 15, 2020
Merged

Add TestGenerateMojo to the akka-grpc-maven-plugin#1100
raboof merged 1 commit intoakka:masterfrom
samantmaharaj:maven-plugin-test-generate

Conversation

@samantmaharaj
Copy link
Copy Markdown
Contributor

References #1099

* Refactor common code to AbstractGenerateMojo
* Introduce TestGenerateMojo including plugin.xml addition
* Introduce new parameter 'outputDirectory' for both GenerateMojo and TestGenerateMojo
@lightbend-cla-validator
Copy link
Copy Markdown

Hi @samantmaharaj,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement:

https://www.lightbend.com/contribute/cla

@akka-ci
Copy link
Copy Markdown

akka-ci commented Aug 14, 2020

Thank you for your pull request! After a quick sanity check one of the team will reply with 'OK TO TEST' to kick off our automated validation on Jenkins. This compiles the project, runs the tests, and checks for things like binary compatibility and source code formatting. When two team members have also manually reviewed and (perhaps after asking for some amendments) accepted your contribution, it should be good to be merged.

For more details about our contributing process, check out CONTRIBUTING.md - and feel free to ask!

@raboof
Copy link
Copy Markdown
Contributor

raboof commented Aug 14, 2020

OK TO TEST

Copy link
Copy Markdown
Contributor

@raboof raboof left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! This mostly LGTM, though it's a shame about the duplication on the XML side. Should we look into other code-generating plugins and see if they have some clever solution for this? Or is this just a limitation of maven?

<generatorSettings implementation="java.util.List" default-value=""/>
<extraGenerators implementation="java.util.List" default-value=""/>
<protoPaths default-value="src/test/proto,src/test/protobuf">${akka-grpc.protoPaths}</protoPaths>
<outputDirectory default-value="target/generated-test-sources">${akka-grpc.outputDirectory}</outputDirectory>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmm, that's quite some duplication.

Is this the way other code-generating plugins do it as well?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If you develop using maven, the plugin.xml is generated based on annotations in the code so it's not a problem that normally comes up as you don't need to maintain it.

I could factor the common XML out and use a templating tool but I'm not that familiar with SBT so I'd appreciate some help on how to do it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Aah makes sense. Adding templating would be possible for example with sbt-twirl, but to be honest I'm not sure the added complexity is worth it. Let's live with the duplication for now ;)

import javax.inject.Inject
import org.sonatype.plexus.build.incremental.BuildContext

class TestGenerateMojo @Inject() (buildContext: BuildContext) extends AbstractGenerateMojo(buildContext) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

@raboof raboof merged commit 4698e1e into akka:master Aug 15, 2020
@samantmaharaj samantmaharaj deleted the maven-plugin-test-generate branch August 15, 2020 20:39
@samantmaharaj samantmaharaj restored the maven-plugin-test-generate branch August 24, 2020 22:12
@raboof raboof changed the title Add TestGenerateMojo to the akka-grpc-maven-plugin #1099 Add TestGenerateMojo to the akka-grpc-maven-plugin Aug 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants