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

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

Merged
merged 4 commits into from
Aug 3, 2018

Conversation

RomainMuller
Copy link
Contributor

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

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

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

@@ -77,14 +77,35 @@ process.stdout.write(`

<dependencies>
<dependency>
<groupId>com.amazonaws</groupId>
<groupId>software.amazon.jsii</groupId>
Copy link

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

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
RomainMuller pushed a commit that referenced this pull request Aug 8, 2018
* 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
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants