Skip to content

Stop using . in artifactId, use - instead, per the usual conventions.#490

Merged
RomainMuller merged 4 commits intomasterfrom
rmuller/pom-convention
Aug 3, 2018
Merged

Stop using . in artifactId, use - instead, per the usual conventions.#490
RomainMuller merged 4 commits intomasterfrom
rmuller/pom-convention

Conversation

@RomainMuller
Copy link
Contributor

@RomainMuller RomainMuller commented Aug 2, 2018

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

@RomainMuller RomainMuller requested a review from kiiadi August 2, 2018 17:27
@kiiadi
Copy link

kiiadi commented Aug 2, 2018

I still think we should drop the aws- prefix from the artifactId, it seems redundant. The AWS SDK for Java v2 just uses the basic service name, so for example the dependency is software.amazon.awssdk:s3:$VERSION. I understand there may be cases where a conflict could arise - but we should tackle the differentiation at that point rather than make every service suffer from the additional redundancy.

@RomainMuller
Copy link
Contributor Author

RomainMuller commented Aug 2, 2018

Discussed off-line with @kiiadi and came to the following conclusion:

  • Drop the aws- prefix from the artifactId will make the maven coordinates simpler & align with the Java SDK 2.0 (maybe allow it as a tie-breaker, but we don't actually have a conflict yet, so probably not)
  • Package names for L1/L2 libraries should be software.amazon.awscdk.services.<package name minus '@aws-cdk/aws-' prefix>
  • It might be a good idea to rename @aws-cdk/rtv to @aws-cdk/runtime-values, because the acronym is not standard & could confuse.

@RomainMuller RomainMuller force-pushed the rmuller/pom-convention branch from 1f07d50 to 8f452df Compare August 2, 2018 18:44
@RomainMuller
Copy link
Contributor Author

RomainMuller commented Aug 2, 2018

Updated PR to change artifactIds and package names as described above, and added a cdk- prefix to CDK core packages.

kiiadi
kiiadi approved these changes Aug 2, 2018
View reviewed changes
examples/cdk-examples-java/pom.xml.t.js
<dependencies>
<dependency>
<groupId>com.amazonaws</groupId>
<groupId>software.amazon.jsii</groupId>
Copy link

@kiiadi kiiadi Aug 2, 2018

Choose a reason for hiding this comment

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

out of interest - how are we releasing jsii? I noticed from the repo there that it's org.jsii - did we want to keep that? or is this an amazon-branded product?

Copy link
Contributor Author

@RomainMuller RomainMuller Aug 3, 2018

Choose a reason for hiding this comment

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

It'll be published under software.amazon.jsii, and supposedly all places were updated accordingly (somehow this one seems to have slept though).

@RomainMuller RomainMuller merged commit a449907 into master Aug 3, 2018
@RomainMuller RomainMuller deleted the rmuller/pom-convention branch August 3, 2018 07:41
@eladb eladb mentioned this pull request Aug 8, 2018
Merged
RomainMuller pushed a commit that referenced this pull request Aug 8, 2018
@RomainMuller
v0.8.1 (#523)
db2ac62 
* Bump
* Added a `bump.sh` script ;-)

Fixes #467 (via aws/jsii#139)
Fixes #503 (via #517)
Fixes #502 (via #490)
@NGL321 NGL321 added the contribution/core This is a PR that came from AWS. label Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rix0rrr rix0rrr rix0rrr approved these changes

+1 more reviewer

@kiiadi kiiadi kiiadi approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

contribution/core This is a PR that came from AWS.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@RomainMuller @kiiadi @rix0rrr @NGL321