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

darwin: use Go type wrappers to avoid declaring Go methods on C types. #163

Merged
merged 4 commits into from
Aug 10, 2023

Conversation

deadprogram
Copy link
Contributor

This is needed starting with Go 1.21 due to stricter enforcement of rules about adding methods to existing types, now including C types. It should fix #162

See golang/go#60725 for further details.

@deadprogram
Copy link
Contributor Author

cc @cmaglie 😸

deadprogram added a commit to tinygo-org/tinygo that referenced this pull request Jul 26, 2023
…go-serial#163 is merged

Signed-off-by: deadprogram <ron@hybridgroup.com>
@deadprogram
Copy link
Contributor Author

Sorry to be a pest but Go 1.21 rc4 is now out, so probably the final release is coming anytime. Any chance to getting this reviewed, please? 😸

cmaglie
cmaglie previously requested changes Aug 3, 2023
Copy link
Member

@cmaglie cmaglie left a comment

Choose a reason for hiding this comment

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

Hi @deadprogram, thank you very much for the PR!

I've posted some change requests to improve readability, I think we could simplify the overall patch. Let me know what do you think.

enumerator/usb_darwin.go Outdated Show resolved Hide resolved
enumerator/usb_darwin.go Outdated Show resolved Hide resolved
enumerator/usb_darwin.go Outdated Show resolved Hide resolved
@cmaglie cmaglie self-assigned this Aug 3, 2023
@cmaglie
Copy link
Member

cmaglie commented Aug 7, 2023

Hi @deadprogram

I've pushed more commits with the changes I proposed in this PR. I tested it and seems to work fine.

I'll keep the change here for a couple of days for you to check, and eventually merge them.

@deadprogram
Copy link
Contributor Author

Thanks for the refinements @cmaglie at first glance it looks good. I will check it out in more detail this evening and get back to you.

@deadprogram
Copy link
Contributor Author

Thank you for the additions @cmaglie let's merge and release please!

deadprogram and others added 4 commits August 10, 2023 13:10
This is needed starting with Go 1.21 due to stricter enforcement of rules
about adding methods to existing types, now including C types.

See golang/go#60725 for further details.

Signed-off-by: deadprogram <ron@hybridgroup.com>
This makes easier the boxing/unboxing of the wrapped types.
@cmaglie cmaglie merged commit 2e90307 into bugst:master Aug 10, 2023
5 of 8 checks passed
@cmaglie
Copy link
Member

cmaglie commented Aug 10, 2023

Here we go: https://github.com/bugst/go-serial/releases/tag/v1.6.0

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.

Doesn't compile with Go 1.21 on macOS
2 participants