Skip to content

changing Interface class name to Abstract#99

Merged
franciscolopezsancho merged 2 commits intolightbend:mainfrom
franciscolopezsancho:renaming-interface
Jul 5, 2021
Merged

changing Interface class name to Abstract#99
franciscolopezsancho merged 2 commits intolightbend:mainfrom
franciscolopezsancho:renaming-interface

Conversation

@franciscolopezsancho
Copy link
Copy Markdown
Contributor

@franciscolopezsancho franciscolopezsancho commented Jul 2, 2021

tested on

  • codegen
  • maven-java
  • single application calling to codegen through maven-java-plugin (maybe unnecessary : )

tested on
- codegen
- maven-java
- single application calling to codegen through maven-java-plugin
@franciscolopezsancho
Copy link
Copy Markdown
Contributor Author

franciscolopezsancho commented Jul 2, 2021

I think we have some dependency issue. The maven-java-test that is failing is because it's using the previous codegen which creates [info] Generated: target/generated-sources/akkaserverless/java/com/example/shoppingcart/domain/ShoppingCartInterface.java . Part of this PR is to make sure the maven-java test now expects target/generated-sources/akkaserverless/java/com/example/shoppingcart/domain/AbstractShopping.java assuming it's using the codegen with the change in this PR.

Shall I do this in two steps? Meaning first a PR only to the codegen and later to maven-java?

Or you might prefer to change the CI so it creates first the codegen and uses it?

LMKWYT

@franciscolopezsancho
Copy link
Copy Markdown
Contributor Author

franciscolopezsancho commented Jul 5, 2021

forget about my last comment, please. Everything should now work, I added the required git dependencies.
#101

@ennru
Copy link
Copy Markdown
Member

ennru commented Jul 5, 2021

Fixes #42

Copy link
Copy Markdown
Member

@ennru ennru left a comment

Choose a reason for hiding this comment

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

LGTM.
A next step could be to remove the Impl postfix on the class created for the user.

@franciscolopezsancho
Copy link
Copy Markdown
Contributor Author

you mean to remove the Impl in this PR @ennru ?

@ennru
Copy link
Copy Markdown
Member

ennru commented Jul 5, 2021

you mean to remove the Impl in this PR

No, let's do that in another PR.

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