Skip to content

Conversation

@scottfeldman
Copy link
Contributor

Hi @paralin, I hope you don't mind me resurrecting PR#24.

This is a rework of PR#24 based on review comments. The rework mostly involved moving changes to the proper files, aligning with upstream Go src/net file layout.

I tested this PR on examples/net examples. No issues, but those tests don't hit the newly added changes, so not expecting any issues. Code changes by inspection LGTM.

If this PR is merged, PR#24 can be closed.

…o-org#2

This is a rework of PR#24 based on review comments.  The rework mostly
involved moving changes to the proper files, aligning with upstream Go
src/net file layout.
@paralin
Copy link
Contributor

paralin commented Jan 5, 2025

Lgtm 👍

@deadprogram
Copy link
Member

Thanks for the addition @paralin and to @scottfeldman for bringing it back to life. Now merging.

@deadprogram deadprogram merged commit 4927c84 into tinygo-org:main Jan 7, 2025
@deadprogram
Copy link
Member

@scottfeldman and @paralin looks like a small problem with Windows and this PR. Please see error here https://github.com/tinygo-org/tinygo/actions/runs/12651738880/job/35253726771?pr=4684#step:7:13

@paralin
Copy link
Contributor

paralin commented Jan 7, 2025

@scottfeldman it looks like lookupProtocol needs to be implemented for the windows build tag.

scottfeldman added a commit to scottfeldman/tinygo-net that referenced this pull request Jan 19, 2025
This fixes a problem with tinygo-org#36
which causes the Windows TinyGO build to fail.  The problem is the
function lookupProtocol was missing for Windows build.  This PR adds the
function for Windows.

I can't really test this because I don't have a Windows system, but by
inspection is seems like it's correct.
deadprogram pushed a commit that referenced this pull request Feb 17, 2025
This fixes a problem with #36
which causes the Windows TinyGO build to fail.  The problem is the
function lookupProtocol was missing for Windows build.  This PR adds the
function for Windows.

I can't really test this because I don't have a Windows system, but by
inspection is seems like it's correct.
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