You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by GitBox <gi...@apache.org> on 2021/09/19 06:04:15 UTC
[GitHub] [thrift] balazsgrill opened a new pull request #2455: Modified build tags to allow js/wasm target
balazsgrill opened a new pull request #2455:
URL: https://github.com/apache/thrift/pull/2455
I tried the golang implementation to be used for wasm platform, and found that I can do so by disabling the unix (not windows) specific `checkConn` and `read0` implementations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] balazsgrill commented on a change in pull request #2455: Modified build tags to allow js/wasm target
Posted by GitBox <gi...@apache.org>.
balazsgrill commented on a change in pull request #2455:
URL: https://github.com/apache/thrift/pull/2455#discussion_r712453129
##########
File path: lib/go/thrift/socket_windows_conn.go
##########
@@ -1,4 +1,4 @@
-// +build windows
+// +build windows wasm
Review comment:
I ran gofmt on the affected files. It also kept the original build tags, which I guess is okay to be compatible with go 1.16 (as stated by the [go.mod](https://github.com/apache/thrift/blob/master/go.mod) file)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] fishy commented on a change in pull request #2455: Modified build tags to allow js/wasm target
Posted by GitBox <gi...@apache.org>.
fishy commented on a change in pull request #2455:
URL: https://github.com/apache/thrift/pull/2455#discussion_r711865341
##########
File path: lib/go/thrift/socket_windows_conn.go
##########
@@ -1,4 +1,4 @@
-// +build windows
+// +build windows wasm
Review comment:
yes please.
while you are here, could you please also run `gofmt` from 1.17+ on these 2 files? go 1.17 introduced new build constraint format, and `gofmt` 1.17+ would auto generate the new build constraint from the old one. (the new one starts with `//go:build` instead of `// +build`, after running `gofmt` both should be there).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] fishy commented on pull request #2455: Modified build tags to allow js/wasm target
Posted by GitBox <gi...@apache.org>.
fishy commented on pull request #2455:
URL: https://github.com/apache/thrift/pull/2455#issuecomment-923282614
Looks like we disabled travis, but I verified this locally with `make check` under `lib/go` and `test/go`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] balazsgrill commented on a change in pull request #2455: Modified build tags to allow js/wasm target
Posted by GitBox <gi...@apache.org>.
balazsgrill commented on a change in pull request #2455:
URL: https://github.com/apache/thrift/pull/2455#discussion_r711864539
##########
File path: lib/go/thrift/socket_windows_conn.go
##########
@@ -1,4 +1,4 @@
-// +build windows
+// +build windows wasm
Review comment:
Done. Do you need me to manually sqash the commits?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] fishy commented on a change in pull request #2455: Modified build tags to allow js/wasm target
Posted by GitBox <gi...@apache.org>.
fishy commented on a change in pull request #2455:
URL: https://github.com/apache/thrift/pull/2455#discussion_r711773344
##########
File path: lib/go/thrift/socket_windows_conn.go
##########
@@ -1,4 +1,4 @@
-// +build windows
+// +build windows wasm
Review comment:
with this change it's better to rename the file as well. maybe `socket_non_unix_conn.go`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [thrift] fishy merged pull request #2455: Modified build tags to allow js/wasm target
Posted by GitBox <gi...@apache.org>.
fishy merged pull request #2455:
URL: https://github.com/apache/thrift/pull/2455
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@thrift.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org