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

bindings: address review comments #20

Merged
merged 4 commits into from
Jun 29, 2023
Merged

bindings: address review comments #20

merged 4 commits into from
Jun 29, 2023

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Jun 26, 2023

Description of Changes

Addresses review comments in #8.

API

  • This is a breaking change to the module API
  • This is a breaking change to the ClientAPI

If the API is breaking, please state below what will break

The constants in the errnos module were renamed.

Copy link
Contributor

@gefjon gefjon left a comment

Choose a reason for hiding this comment

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

This is excellent!

@Centril Centril mentioned this pull request Jun 27, 2023
2 tasks
Copy link
Collaborator

@jdetter jdetter left a comment

Choose a reason for hiding this comment

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

LGTM 👍

/// This allows later changing the meaning of the returned type variable
/// if for whatever reason, you cannot provide the full definition of the type yet.
/// This allows for self referential,
/// recursive or other complex types to be declared in the typespace.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Much improved, thank you 🙏

@Centril Centril enabled auto-merge (squash) June 29, 2023 14:21
@Centril Centril merged commit 073889f into master Jun 29, 2023
cloutiertyler pushed a commit that referenced this pull request Aug 1, 2023
* integrate jdetter's review

* address phoebe's comments

* improve errnos naming
cloutiertyler pushed a commit that referenced this pull request Aug 1, 2023
* integrate jdetter's review

* address phoebe's comments

* improve errnos naming
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.

3 participants