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

Use unsafe.Slice instead of reflect.SliceHeader to build byte slice #65

Merged
merged 1 commit into from
Aug 27, 2024

Conversation

inliquid
Copy link
Contributor

@inliquid inliquid commented Aug 24, 2024

Issue #, if available:
#64

Description of changes:
Use unsafe.Slice instead of reflect.SliceHeader to build byte slice

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@inliquid
Copy link
Contributor Author

@evacchi sorry for tagging, but could you please take a look, as the linked issue prevents us from using recent versions of tinygo (v0.32.0 and v0.33.0).

Copy link
Contributor

@evacchi evacchi left a comment

Choose a reason for hiding this comment

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

@inliquid
Copy link
Contributor Author

Thank you for a quick review! Looking forward for a merge 😊

@knqyf263
Copy link
Owner

I triggered tests.

@inliquid
Copy link
Contributor Author

I triggered tests.

@knqyf263 sorry, I missed this somehow. I tested on a different environment (including compilation with tinygo v0.31.*) where import was removed by IDE, but forgot this part when was preparing PR. Should be fixed now

Copy link
Owner

@knqyf263 knqyf263 left a comment

Choose a reason for hiding this comment

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

Thanks

@knqyf263 knqyf263 merged commit 114c625 into knqyf263:main Aug 27, 2024
3 checks passed
@inliquid
Copy link
Contributor Author

@knqyf263 can you please tag a new version?

@knqyf263
Copy link
Owner

Yes, but the main branch is failing...
https://github.com/knqyf263/go-plugin/actions/runs/10570578373/job/29285270109

@inliquid
Copy link
Contributor Author

Thanks! I see, I will analyze it and open an issue.

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