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.

Sign up for GitHub

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

Jump to bottom

feat(java): add a Builder<T> interface implemented by all builders #1654

Merged
merged 3 commits into from
May 13, 2020
Merged

feat(java): add a Builder<T> interface implemented by all builders #1654

merged 3 commits into from
May 13, 2020

Conversation

RomainMuller
Copy link
Contributor

@RomainMuller RomainMuller commented May 12, 2020 •
edited
Loading

Commit Message

feat(java): add a Builder interface implemented by all builders (#1654)

The common super-interface for all Builder classes creates a nice and
clean extension point that can be leveraged to improve the experience of
developers using Kotlin extensions, or doing fancy AOP things. For example,
this enables introduction of Kotlin extensions to improve/simplify the syntax
needed to initialize types that provide a Builder further from what the
Java implementation natively allows (see the related issue for more info).

The new interface does not change the current Builder layout, merely
adding the new super-interface to pre-declare an already-existing method.

Fixes #1652

End Commit Message

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

@RomainMuller
feat(java): add a Builder<T> interface implemented by all builders
ef06a5c 
The common super-interface for all `Builder` classes creates a nice and
clean extension point that can be leveraged to improve the experience of
developers using Kotlin extensions, or doing fancy AOP things.

The new interface does not change the current `Builder` layout, merely
adding the new super-interface to pre-declare an already-existing method.

Fixes #1652
@RomainMuller RomainMuller requested a review from a team May 12, 2020 14:12
@RomainMuller RomainMuller self-assigned this May 12, 2020
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label May 12, 2020
@RomainMuller RomainMuller added language/java Related to Java bindings module/pacmak Issues affecting the `jsii-pacmak` module effort/small Small work item – less than a day of effort labels May 12, 2020
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-Blkkw9bQFn8A
  • Commit ID: ef06a5c
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@nija-at nija-at added the pr/do-not-merge This PR should not be merged at this time. label May 13, 2020
nija-at
nija-at approved these changes May 13, 2020
View reviewed changes
Copy link
Contributor

@nija-at nija-at left a comment

Choose a reason for hiding this comment

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

LGTM!
Added 'do-not-merge' in case you want someone more familiar to look at this.

Would prefer if you can be more clear about - "improve the experience of
developers using Kotlin extensions, or doing fancy AOP things." - in the commit message.
I got the answer to this from the attached issue, but summarizing this in the commit message would be better.

packages/@jsii/java-runtime/project/src/main/java/software/amazon/jsii/Builder.java Outdated Show resolved Hide resolved
@RomainMuller
Update packages/@jsii/java-runtime/project/src/main/java/software/ama…
47d51a2 
…zon/jsii/Builder.java

Co-authored-by: Niranjan Jayakar <nija@amazon.com>
@RomainMuller RomainMuller removed the pr/do-not-merge This PR should not be merged at this time. label May 13, 2020
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-Blkkw9bQFn8A
  • Commit ID: 47d51a2
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Contributor

mergify bot commented May 13, 2020

Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it!

@mergify mergify bot added the pr/ready-to-merge This PR is ready to be merged. label May 13, 2020
@mergify
Copy link
Contributor

mergify bot commented May 13, 2020

Merging (with squash)...

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-Blkkw9bQFn8A
  • Commit ID: 664b669
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify mergify bot merged commit 52e73b5 into master May 13, 2020
@mergify mergify bot deleted the rmuller/java-builder-interface branch May 13, 2020 17:40
@mergify
Copy link
Contributor

mergify bot commented May 13, 2020

Merging (with squash)...

@mergify mergify bot removed the pr/ready-to-merge This PR is ready to be merged. label May 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nija-at nija-at nija-at approved these changes

Assignees

@RomainMuller RomainMuller

Labels
contribution/core This is a PR that came from AWS. effort/small Small work item – less than a day of effort language/java Related to Java bindings module/pacmak Issues affecting the `jsii-pacmak` module
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Java APIs: Add a Builder interface implemented by all Builders
3 participants
@RomainMuller @aws-cdk-automation @nija-at