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

refactor(go): updates to RFC204 - golang bindings #292

Merged
merged 5 commits into from
Mar 5, 2021

Conversation

RomainMuller
Copy link
Contributor

@RomainMuller RomainMuller commented Feb 23, 2021

This updates RFC204 based on the latest developments for the
implementation thereof. In particular, the following elements have been
changed:

  • Updates to jsii enums

    • Preserve capitalisation of enum constant names, and separate from type name with _
  • Updates to jsii interfaces (aka: behavioral interfaces)

    • Added a go struct for the concrete jsii proxy object
  • Updates to jsii structs (aka: datatype interfaces)

    • Replaced Iface suffix in go interface names with an I prefix
    • Added Approach 3 for handling jsii struct inheritance
  • Updates to jsii classes

    • Removed Iface suffix from go interfaces generated for classes
    • Un-exported the go struct concrete type for the jsii proxy objects
    • Removed unused properties from concrete types for jsii proxy objects
    • Used mock jsii runtime calls to clarify activities that are proxied
      to the @jsii/kernel process
    • Using _ to separate the type name & static method or property name
  • Updates to properties:

    • Where possible (i.e: everywhere, except in structs approaches 1, 2 and 3,
      removed the Get prefix from getters (keeping Set prefix for setters), to
      match the recommendations of effective go

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

This updates RFC204 based on the latest developments for the
implementation thereof. In particular, the following elements have been
changed:

- Updates to *jsii interfaces* (aka: behavioral interfaces)
  - Added a go struct for the concrete jsii proxy object

- Updates to *jsii structs* (aka: datatype interfaces)
  - Replaced `Iface` suffix in go interface names with an `I` prefix
  - Added *Approach 3* for handling jsii struct inheritance

- Updates to *jsii classes*
  - Removed `Iface` suffix from go interfaces generated for classes
  - Un-exported the go struct concrete type for the jsii proxy objects
  - Removed unused properties from concrete types for jsii proxy objects
  - Used mock jsii runtime calls to clarify activities that are proxied
    to the `@jsii/kernel` process
@RomainMuller RomainMuller added jsii status/review Proposal pending review/revision contribution/core This is a PR that came from AWS labels Feb 23, 2021
@RomainMuller RomainMuller requested a review from a team February 23, 2021 10:32
@RomainMuller RomainMuller self-assigned this Feb 23, 2021
Copy link
Contributor

@eladb eladb left a comment

Choose a reason for hiding this comment

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

Overall looks much better from the DevX point of view. I am not sure I fully understand the convoluted setup around data types. I'll follow up face to face

text/204-golang-bindings.md Outdated Show resolved Hide resolved
text/204-golang-bindings.md Show resolved Hide resolved
text/204-golang-bindings.md Show resolved Hide resolved
text/204-golang-bindings.md Outdated Show resolved Hide resolved
Copy link
Contributor

@eladb eladb left a comment

Choose a reason for hiding this comment

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

👍

}

// Optional - we can reduce boilerplate for conversion of BaseServiceProps -> BaseServiceOptions:
func (b BaseServiceProps) ToBaseServiceOptions() BaseServiceOptions {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice touch

RomainMuller added a commit to aws/jsii that referenced this pull request Feb 23, 2021
As suggested in aws/aws-cdk-rfcs#292, using `_` to separate the constant
name from the type name, and preserving the constant's casing results in
an API that is slightly more readable, and less prone to namespace
collisions.
RomainMuller added a commit to aws/jsii that referenced this pull request Feb 23, 2021
As proposed in aws/aws-cdk-rfcs#292 (this is *Approach 4*), stop
rendering go interfaces to represent jsii structs, and instead only emit
a plain go struct with the flattened list of fields (own + all super
interfaces).

Made the necessary code changes to de-serialize structs returned
by-reference by the `@jsii/kernel` by eagerly fecthing all properties.

Also, implemented the option to offer convenience conversion functions
to easily create a parent type from a child type.
RomainMuller added a commit to aws/jsii that referenced this pull request Feb 25, 2021
As suggested in aws/aws-cdk-rfcs#292, using `_` to separate the constant
name from the type name, and preserving the constant's casing results in
an API that is slightly more readable, and less prone to namespace
collisions.
RomainMuller added a commit to aws/jsii that referenced this pull request Feb 25, 2021
As proposed in aws/aws-cdk-rfcs#292 (this is *Approach 4*), stop
rendering go interfaces to represent jsii structs, and instead only emit
a plain go struct with the flattened list of fields (own + all super
interfaces).

Made the necessary code changes to de-serialize structs returned
by-reference by the `@jsii/kernel` by eagerly fecthing all properties.

Also, implemented the option to offer convenience conversion functions
to easily create a parent type from a child type.
This was referenced Mar 17, 2021
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 jsii status/review Proposal pending review/revision
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants