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 2021/02/19 18:32:55 UTC

[GitHub] [skywalking-satellite] EvanLjp opened a new pull request #27: polish codes for release & add performance test doc

EvanLjp opened a new pull request #27:
URL: https://github.com/apache/skywalking-satellite/pull/27


   This is maybe the last PR for 0.0.1 milestone. Thx for reviewing it.


----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] kezhenxu94 commented on a change in pull request #27: polish codes for release & add performance test doc

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #27:
URL: https://github.com/apache/skywalking-satellite/pull/27#discussion_r579603854



##########
File path: plugins/queue/mmap/queue.go
##########
@@ -87,17 +87,15 @@ func (q *Queue) Description() string {
 func (q *Queue) DefaultConfig() string {
 	return `
 # The size of each segment. Default value is 128K. The unit is Byte.
-segment_size: 131072
+segment_size: 262114

Review comment:
       Ditto

##########
File path: docs/en/setup/plugins/queue_mmap-queue.md
##########
@@ -4,17 +4,15 @@ This is a memory mapped queue to provide the persistent storage for the input ev
 ## DefaultConfig
 ```yaml
 # The size of each segment. Default value is 128K. The unit is Byte.
-segment_size: 131072
+segment_size: 262114

Review comment:
       do you mean `262144`? `262144 == 256K` while `262114` is odd. Also need to update the comment above (line 6)




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] kezhenxu94 commented on a change in pull request #27: polish codes for release & add performance test doc

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #27:
URL: https://github.com/apache/skywalking-satellite/pull/27#discussion_r579590515



##########
File path: Makefile
##########
@@ -73,11 +75,16 @@ clean: tools
 	-rm -rf coverage.txt
 
 .PHONY: build
-build: clean deps linux darwin windows
+build: clean deps
+	rm -rf bin/*

Review comment:
       I polish the docker image, please review https://github.com/apache/skywalking-satellite/pull/28 and rebase this on that after merging it




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] kezhenxu94 edited a comment on pull request #27: polish codes for release & add performance test doc

Posted by GitBox <gi...@apache.org>.
kezhenxu94 edited a comment on pull request #27:
URL: https://github.com/apache/skywalking-satellite/pull/27#issuecomment-782577735


   > > should we add some e2e test cases before release ?
   > 
   > @kezhenxu94 Do we have some ways to do it?
   
   The added docker-compose.yaml in #28 can be used to do E2E tests, @Humbertzhang @fgksgf do you have time to try to set it up in this repo? Maybe using the NGE2E


----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] kezhenxu94 commented on pull request #27: polish codes for release & add performance test doc

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on pull request #27:
URL: https://github.com/apache/skywalking-satellite/pull/27#issuecomment-782577735


   > > should we add some e2e test cases before release ?
   > 
   > @kezhenxu94 Do we have some ways to do it?
   
   The added docker-compose.yaml in #28 can be used to do E2E tests, @Humbertzhang @fgksgf do you have time to try to set it up in this repo?


----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] wu-sheng commented on pull request #27: polish codes for release & add performance test doc

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #27:
URL: https://github.com/apache/skywalking-satellite/pull/27#issuecomment-782474142


   > This is maybe the last PR for 0.0.1 milestone. Thx for reviewing it.
   
   Version incorrect. Your release is 0.1.0. The last version number is for bug hot-fix(emergency) only in 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



[GitHub] [skywalking-satellite] EvanLjp commented on pull request #27: polish codes for release & add performance test doc

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on pull request #27:
URL: https://github.com/apache/skywalking-satellite/pull/27#issuecomment-782577287


   > should we add some e2e test cases before release ?
   
   @kezhenxu94 Do we have some ways to do it?


----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #27: polish codes for release & add performance test doc

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #27:
URL: https://github.com/apache/skywalking-satellite/pull/27#discussion_r579563610



##########
File path: Makefile
##########
@@ -73,11 +75,16 @@ clean: tools
 	-rm -rf coverage.txt
 
 .PHONY: build
-build: clean deps linux darwin windows
+build: clean deps
+	rm -rf bin/*

Review comment:
       In the local test , I build some different version docker images and push them to a private repository. Because of using the find command in the start script, the docker started failed when having history file. If needed to the original version, I would rollback it.




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] kezhenxu94 commented on a change in pull request #27: polish codes for release & add performance test doc

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #27:
URL: https://github.com/apache/skywalking-satellite/pull/27#discussion_r579521031



##########
File path: Makefile
##########
@@ -73,11 +75,16 @@ clean: tools
 	-rm -rf coverage.txt
 
 .PHONY: build
-build: clean deps linux darwin windows
+build: clean deps
+	rm -rf bin/*

Review comment:
       This line should be in `clean` goal?




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] wu-sheng merged pull request #27: polish codes for release & add performance test doc

Posted by GitBox <gi...@apache.org>.
wu-sheng merged pull request #27:
URL: https://github.com/apache/skywalking-satellite/pull/27


   


----------------------------------------------------------------
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