You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "hxmhlt (GitHub)" <gi...@apache.org> on 2020/02/29 04:32:50 UTC
[GitHub] [dubbo-go] hxmhlt opened pull request #298: Ftr: add
protobuf support
<!-- Thanks for sending a pull request!
-->
**What this PR does**:
- support protobuf
- some code refactoring, add `Serializer` interface and serialization layer
**What this PR fix**:
- registry url should not be encoded twice in `zookeeper/registry.go`
**Special notes for your reviewer**: file changes maybe too much, please be patient
**Does this PR introduce a user-facing change?**: No, all things are compatible.
<!--
If no, just write "NONE" in the release-note block below.
If yes, a release note is required:
Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".
-->
ps: protoc-gen-dubbogo plugin are in the [repo](https://github.com/skyitachi/protoc-gen-dubbogo)
[ Full content available at: https://github.com/apache/dubbo-go/pull/298 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [dubbo-go] skyitachi commented on pull request #298: Ftr:
add protobuf support
Posted by "skyitachi (GitHub)" <gi...@apache.org>.
I know，but dubbo-go-hessian2 do a lot more work other than serialize include dubbo codec，so I use dubbo codec directly and go-hessian2 as pure serializer，just as what does in dubbo java，keep go-hessian2’s serialize functions
[ Full content available at: https://github.com/apache/dubbo-go/pull/298 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [dubbo-go] skyitachi commented on pull request #298: Ftr:
add protobuf support
Posted by "skyitachi (GitHub)" <gi...@apache.org>.
same as above
[ Full content available at: https://github.com/apache/dubbo-go/pull/298 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [dubbo-go] skyitachi commented on issue #298: Ftr: add
protobuf support
Posted by "skyitachi (GitHub)" <gi...@apache.org>.
I think should make a new pr :)
[ Full content available at: https://github.com/apache/dubbo-go/pull/298 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [dubbo-go] xujianhai666 commented on issue #298: Ftr: add
protobuf support
Posted by "xujianhai666 (GitHub)" <gi...@apache.org>.
@skyitachi I thought we should discuss with @AlexStocks . If we should move hessian from hessian repo to here, we should seperate those two work.
[ Full content available at: https://github.com/apache/dubbo-go/pull/298 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [dubbo-go] xujianhai666 commented on pull request #298: Ftr:
add protobuf support
Posted by "xujianhai666 (GitHub)" <gi...@apache.org>.
no chinese comment here, pls remove
[ Full content available at: https://github.com/apache/dubbo-go/pull/298 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [dubbo-go] xujianhai666 commented on pull request #298: Ftr:
add protobuf support
Posted by "xujianhai666 (GitHub)" <gi...@apache.org>.
chinese comment， u know
[ Full content available at: https://github.com/apache/dubbo-go/pull/298 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [dubbo-go] AlexStocks commented on issue #298: Ftr: add
protobuf support
Posted by "AlexStocks (GitHub)" <gi...@apache.org>.
> @skyitachi I thought we should discuss with @AlexStocks . If we should move hessian from hessian repo to here, we should seperate those two work.
u should have a discuss with @hxmhlt
[ Full content available at: https://github.com/apache/dubbo-go/pull/298 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [dubbo-go] AlexStocks commented on issue #298: Ftr: add
protobuf support
Posted by "AlexStocks (GitHub)" <gi...@apache.org>.
@skyitachi pls check travis result.
[ Full content available at: https://github.com/apache/dubbo-go/pull/298 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [dubbo-go] hxmhlt commented on pull request #298: Ftr: add
protobuf support
Posted by "hxmhlt (GitHub)" <gi...@apache.org>.
import fmt
[ Full content available at: https://github.com/apache/dubbo-go/pull/298 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [dubbo-go] xujianhai666 commented on pull request #298: Ftr:
add protobuf support
Posted by "xujianhai666 (GitHub)" <gi...@apache.org>.
I found this method is extra, maybe we should delete ?
[ Full content available at: https://github.com/apache/dubbo-go/pull/298 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [dubbo-go] AlexStocks commented on issue #298: Ftr: add
protobuf support
Posted by "AlexStocks (GitHub)" <gi...@apache.org>.
@xujianhai666 any improvement progress recently?
[ Full content available at: https://github.com/apache/dubbo-go/pull/298 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [dubbo-go] xujianhai666 commented on pull request #298: Ftr:
add protobuf support
Posted by "xujianhai666 (GitHub)" <gi...@apache.org>.
maybe we should abstract a interface
[ Full content available at: https://github.com/apache/dubbo-go/pull/298 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [dubbo-go] xujianhai666 commented on pull request #298: Ftr:
add protobuf support
Posted by "xujianhai666 (GitHub)" <gi...@apache.org>.
I thought we should discuss with @AlexStocks, I found hessian codec impl below.
[ Full content available at: https://github.com/apache/dubbo-go/pull/298 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [dubbo-go] AlexStocks commented on pull request #298: Ftr:
add protobuf support
Posted by "AlexStocks (GitHub)" <gi...@apache.org>.
change it to another import block.
[ Full content available at: https://github.com/apache/dubbo-go/pull/298 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [dubbo-go] AlexStocks commented on pull request #298: Ftr:
add protobuf support
Posted by "AlexStocks (GitHub)" <gi...@apache.org>.
the same as above, u should use dubbo-go-hessian2 constants instead.
[ Full content available at: https://github.com/apache/dubbo-go/pull/298 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [dubbo-go] AlexStocks commented on pull request #298: Ftr:
add protobuf support
Posted by "AlexStocks (GitHub)" <gi...@apache.org>.
translate it to english.
[ Full content available at: https://github.com/apache/dubbo-go/pull/298 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [dubbo-go] AlexStocks commented on pull request #298: Ftr:
add protobuf support
Posted by "AlexStocks (GitHub)" <gi...@apache.org>.
hey guy, the above constants have been defined in dubbo-go-hessian2.
[ Full content available at: https://github.com/apache/dubbo-go/pull/298 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [dubbo-go] hxmhlt commented on issue #298: Ftr: add protobuf
support
Posted by "hxmhlt (GitHub)" <gi...@apache.org>.
I suggest that creating a base protocol like 'BaseTCPProtocol' which inherited by dubboProtocol & protobufProtocol is more better .
[ Full content available at: https://github.com/apache/dubbo-go/pull/298 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [dubbo-go] hxmhlt commented on pull request #298: Ftr: add
protobuf support
Posted by "hxmhlt (GitHub)" <gi...@apache.org>.
Why not return Serializer type other than interface{}?
[ Full content available at: https://github.com/apache/dubbo-go/pull/298 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [dubbo-go] xujianhai666 closed pull request #298: Ftr: add
protobuf support
Posted by "xujianhai666 (GitHub)" <gi...@apache.org>.
[ pull request closed by xujianhai666 ]
[ Full content available at: https://github.com/apache/dubbo-go/pull/298 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org