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

fix: use com.akkaserverless groupId and renamed artifacts #45

Merged

Conversation

octonato
Copy link
Member

No description provided.

@octonato octonato requested a review from ennru June 17, 2021 10:15
Copy link
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.

Drop akkaserverless- from the module and artefact names?

.circleci/config.yml Outdated Show resolved Hide resolved
.circleci/config.yml Outdated Show resolved Hide resolved
@@ -1,11 +1,11 @@
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>

<artifactId>maven-archetype-akkasls-event-sourced-entity</artifactId>
<artifactId>akkaserverless-maven-archetype-event-sourced-entity</artifactId>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<artifactId>akkaserverless-maven-archetype-event-sourced-entity</artifactId>
<artifactId>maven-archetype-event-sourced-entity</artifactId>

As we get akkaserverless in the group ID, we could drop it from the artifact ID.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, we said it on the call, but when changing it I found it would be better to keep it. Mainly because of the value-entity one. It feels a bit naked if only maven-archetype for the default one.

But I agree that we are being repetitive when saying com.akkaserverless:akkaserverless-maven-archetype

Copy link
Member Author

Choose a reason for hiding this comment

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

An alternative could be the following:

starter for the the default one
(uses value-entity)

 mvn archetype:generate \                                                                                                                                                                         <rgc/rename-artifacts-for-publishing>
  -DarchetypeGroupId=com.akkaserverless \
  -DarchetypeArtifactId=maven-project-starter \
  -DarchetypeVersion=LATEST

and then...

 mvn archetype:generate \                                                                                                                                                                         <rgc/rename-artifacts-for-publishing>
  -DarchetypeGroupId=com.akkaserverless \
  -DarchetypeArtifactId=maven-project-event-sourced-entity \
  -DarchetypeVersion=LATEST

Then, later on, if we decide that the default is without any proto file, we can keep the name maven-project-starter and we add another one called maven-project-value-entity.

Copy link
Member

Choose a reason for hiding this comment

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

Let's stick to archetype as that is Maven-speak.

We should not introduce another thing like "starter". "kickstart" would align with what we use in the docs right now, but it has been questioned if that is a good word.

I don't see a problem with the naked "maven-archetype", actually.

Copy link
Member Author

Choose a reason for hiding this comment

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

shall we also remove akkaserverless from akkaserverless-maven-plugin?

that will make:

com.akkaserverless % maven-plugin
com.akkaserverless % maven-archetype
com.akkaserverless % maven-archetype-event-sourced-entity

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that should appeal to your need for consistency. ;-)

Copy link
Member Author

@octonato octonato Jun 17, 2021

Choose a reason for hiding this comment

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

well, unfortunately that won't work

Artifact Ids of the format maven-___-plugin are reserved for 
plugins in the Group Id org.apache.maven.plugins
Please change your artifactId to the format ___-maven-plugin
In the future this error will break the build.

plugins starting with maven- are reserved

octonato and others added 2 commits June 17, 2021 14:53
Co-authored-by: Enno Runne <458526+ennru@users.noreply.github.com>
@octonato octonato force-pushed the rgc/rename-artifacts-for-publishing branch from b8a8446 to 0e1edff Compare June 17, 2021 13:00
Copy link
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.

@octonato octonato merged commit 521596f into rgc/feature-branch-maven-java Jun 17, 2021
@octonato octonato deleted the rgc/rename-artifacts-for-publishing branch June 17, 2021 16:15
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.

None yet

2 participants