You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2020/03/27 06:03:14 UTC
[GitHub] [skywalking-data-collect-protocol] heyanlong opened a new pull
request #21: Add golang package
heyanlong opened a new pull request #21: Add golang package
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/21
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking-data-collect-protocol] arugal commented on a change in
pull request #21: Add golang package
Posted by GitBox <gi...@apache.org>.
arugal commented on a change in pull request #21: Add golang package
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/21#discussion_r399272022
##########
File path: browser/BrowserPerf.proto
##########
@@ -22,6 +22,7 @@ package skywalking.network.protocol.browser;
option java_multiple_files = true;
option java_package = "org.apache.skywalking.apm.network.language.agent";
+option go_package = "agent/pb/language/agent";
Review comment:
`go2sky` does not currently refer to this repo through submodule.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking-data-collect-protocol] wu-sheng commented on a change
in pull request #21: Add golang package
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #21: Add golang package
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/21#discussion_r399957372
##########
File path: browser/BrowserPerf.proto
##########
@@ -22,6 +22,7 @@ package skywalking.network.protocol.browser;
option java_multiple_files = true;
option java_package = "org.apache.skywalking.apm.network.language.agent";
+option go_package = "agent/pb/language/agent";
Review comment:
My point is you have `agent` as prefix and suffix. I think the prefix should be skywalking?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking-data-collect-protocol] wu-sheng commented on a change
in pull request #21: Add golang package
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #21: Add golang package
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/21#discussion_r399720636
##########
File path: browser/BrowserPerf.proto
##########
@@ -22,6 +22,7 @@ package skywalking.network.protocol.browser;
option java_multiple_files = true;
option java_package = "org.apache.skywalking.apm.network.language.agent";
+option go_package = "agent/pb/language/agent";
Review comment:
@heyanlong What is your naming rule? They are inconsistent, And I can't see the pattern.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking-data-collect-protocol] wu-sheng commented on a change
in pull request #21: Add golang package
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #21: Add golang package
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/21#discussion_r399312448
##########
File path: browser/BrowserPerf.proto
##########
@@ -22,6 +22,7 @@ package skywalking.network.protocol.browser;
option java_multiple_files = true;
option java_package = "org.apache.skywalking.apm.network.language.agent";
+option go_package = "agent/pb/language/agent";
Review comment:
OK. But I think we should, right?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking-data-collect-protocol] arugal commented on a change in
pull request #21: Add golang package
Posted by GitBox <gi...@apache.org>.
arugal commented on a change in pull request #21: Add golang package
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/21#discussion_r399687021
##########
File path: browser/BrowserPerf.proto
##########
@@ -22,6 +22,7 @@ package skywalking.network.protocol.browser;
option java_multiple_files = true;
option java_package = "org.apache.skywalking.apm.network.language.agent";
+option go_package = "agent/pb/language/agent";
Review comment:
> OK. But I think we should, right?
Agreed.
> What is the status of go2sky? I assume we are not using this package right now. What is the impact?
Now [go2sky](https://github.com/SkyAPM/go2sky/blob/aa3669fff7f880f6592eaacf5e6ca499cd74539d/reporter/grpc/common/CLR.proto#L21-L24) set `go_package` to `go2sky/reporter/grpc`, it can be modified to `agent/pb/language/agent`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking-data-collect-protocol] wu-sheng commented on a change
in pull request #21: Add golang package
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #21: Add golang package
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/21#discussion_r399052277
##########
File path: browser/BrowserPerf.proto
##########
@@ -22,6 +22,7 @@ package skywalking.network.protocol.browser;
option java_multiple_files = true;
option java_package = "org.apache.skywalking.apm.network.language.agent";
+option go_package = "agent/pb/language/agent";
Review comment:
@arugal @hanahmily What is the status of go2sky? I assume we are not using this package right now. What is the impact?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking-data-collect-protocol] heyanlong commented on a change
in pull request #21: Add golang package
Posted by GitBox <gi...@apache.org>.
heyanlong commented on a change in pull request #21: Add golang package
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/21#discussion_r399909992
##########
File path: browser/BrowserPerf.proto
##########
@@ -22,6 +22,7 @@ package skywalking.network.protocol.browser;
option java_multiple_files = true;
option java_package = "org.apache.skywalking.apm.network.language.agent";
+option go_package = "agent/pb/language/agent";
Review comment:
pb is short for protobuf
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking-data-collect-protocol] heyanlong commented on a change
in pull request #21: Add golang package
Posted by GitBox <gi...@apache.org>.
heyanlong commented on a change in pull request #21: Add golang package
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/21#discussion_r399909784
##########
File path: browser/BrowserPerf.proto
##########
@@ -22,6 +22,7 @@ package skywalking.network.protocol.browser;
option java_multiple_files = true;
option java_package = "org.apache.skywalking.apm.network.language.agent";
+option go_package = "agent/pb/language/agent";
Review comment:
> @heyanlong您的命名规则是什么？它们不一致，而且我看不到模式。
From java
network -> pb
language -> language
agent -> agent
and so
projectName/pb/language/agent
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking-data-collect-protocol] heyanlong closed pull request
#21: Add golang package
Posted by GitBox <gi...@apache.org>.
heyanlong closed pull request #21: Add golang package
URL: https://github.com/apache/skywalking-data-collect-protocol/pull/21
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services