You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/11/15 11:32:35 UTC

[GitHub] [apisix-dashboard] gxthrj opened a new pull request #807: chore: rename makefile release-src to release

gxthrj opened a new pull request #807:
URL: https://github.com/apache/apisix-dashboard/pull/807


   Please answer these questions before submitting a pull request
   
   - Why submit this pull request?
   - [ ] Bugfix
   - [ ] New feature provided
   - [ ] Improve performance
   
   - Related issues
   
   ___
   
   


----------------------------------------------------------------
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] [apisix-dashboard] moonming commented on a change in pull request #807: chore: rename makefile release-src to release

Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #807:
URL: https://github.com/apache/apisix-dashboard/pull/807#discussion_r523759395



##########
File path: Makefile
##########
@@ -91,5 +93,10 @@ endif
 
 .PHONY: release
 release:
-	tar –cf dashboard.tar ./output/*
+	tar -zcvf $(RELEASE).tgz ./output/*

Review comment:
       NO.
   `release` is ONLY for release manager, who ONLY release source code packge.




----------------------------------------------------------------
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] [apisix-dashboard] moonming commented on a change in pull request #807: chore: rename makefile release-src to release

Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #807:
URL: https://github.com/apache/apisix-dashboard/pull/807#discussion_r523759395



##########
File path: Makefile
##########
@@ -91,5 +93,10 @@ endif
 
 .PHONY: release
 release:
-	tar –cf dashboard.tar ./output/*
+	tar -zcvf $(RELEASE).tgz ./output/*

Review comment:
       NO.
   `release` is ONLY for release manager, who ONLY release source code package.




----------------------------------------------------------------
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] [apisix-dashboard] moonming commented on a change in pull request #807: chore: rename makefile release-src to release

Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #807:
URL: https://github.com/apache/apisix-dashboard/pull/807#discussion_r524830489



##########
File path: .gitignore
##########
@@ -42,6 +42,9 @@ build
 manager-api
 
 output
+publish

Review comment:
       agreed. @gxthrj please refer to other repos first.
   To be honest, I don’t know what this PR does




----------------------------------------------------------------
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] [apisix-dashboard] moonming commented on a change in pull request #807: chore: rename makefile release-src to release

Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #807:
URL: https://github.com/apache/apisix-dashboard/pull/807#discussion_r524830489



##########
File path: .gitignore
##########
@@ -42,6 +42,9 @@ build
 manager-api
 
 output
+publish

Review comment:
       agreed. @gxthrj please refer to other repos first.
   To be honest, I don’t know what PR does




----------------------------------------------------------------
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] [apisix-dashboard] gxthrj commented on a change in pull request #807: chore: rename makefile release-src to release

Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #807:
URL: https://github.com/apache/apisix-dashboard/pull/807#discussion_r524300851



##########
File path: Makefile
##########
@@ -91,5 +93,10 @@ endif
 
 .PHONY: release
 release:
-	tar –cf dashboard.tar ./output/*
+	tar -zcvf $(RELEASE).tgz ./output/*

Review comment:
       Ok, It seems that `make release` should not be used here.
   The `output` directory is not source code. They are the result of compilation, including manager and web.
   Maybe `make publish` is more suitable. Ref to some web projects , Such as elementUI.




----------------------------------------------------------------
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] [apisix-dashboard] juzhiyuan commented on a change in pull request #807: chore: rename makefile release-src to release

Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #807:
URL: https://github.com/apache/apisix-dashboard/pull/807#discussion_r524823060



##########
File path: .gitignore
##########
@@ -42,6 +42,9 @@ build
 manager-api
 
 output
+publish

Review comment:
       We use `publish` as a folder is not a good idea, can you provide some examples of other popular projects using this name?
   
   The act of ordering this line is to package the product, and we need to think about whether we need to do this here. 
   
   If we need a package as the final product, then just packaged it in the build process.
   If we use a package just to let users move it easily, let's call it `make package`, but I don't think it's needed and will take the confusion with `make build`.
   
   I would prefer packaging in the final build process or just removed this command to reduce the confusion.
   
   cc @moonming @membphis any ideas?




----------------------------------------------------------------
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] [apisix-dashboard] gxthrj commented on pull request #807: chore: rename makefile release-src to release

Posted by GitBox <gi...@apache.org>.
gxthrj commented on pull request #807:
URL: https://github.com/apache/apisix-dashboard/pull/807#issuecomment-727566485


   Add sha512 and mv the pkgs to release directory .
   Refer to apisix's make release.


----------------------------------------------------------------
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] [apisix-dashboard] gxthrj commented on pull request #807: chore: rename makefile release-src to release

Posted by GitBox <gi...@apache.org>.
gxthrj commented on pull request #807:
URL: https://github.com/apache/apisix-dashboard/pull/807#issuecomment-728840679


   Close the PR,  #816 has been merged. 
   It seems no necessary to package the output directory.


----------------------------------------------------------------
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] [apisix-dashboard] gxthrj commented on a change in pull request #807: chore: rename makefile release-src to release

Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #807:
URL: https://github.com/apache/apisix-dashboard/pull/807#discussion_r523748581



##########
File path: Makefile
##########
@@ -89,7 +89,7 @@ endif
 	.actions/openwhisk-utilities/scancode/scanCode.py --config .actions/ASF-Release.cfg ./
 
 
-.PHONY: release-src
-release-src:
+.PHONY: release

Review comment:
       Do you want to include the build process?




----------------------------------------------------------------
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] [apisix-dashboard] juzhiyuan commented on a change in pull request #807: chore: rename makefile release-src to release

Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #807:
URL: https://github.com/apache/apisix-dashboard/pull/807#discussion_r524823060



##########
File path: .gitignore
##########
@@ -42,6 +42,9 @@ build
 manager-api
 
 output
+publish

Review comment:
       We use `publish` as a folder is not a good idea, can you provide some examples of other popular projects?
   
   The act of ordering this line is to package the product, and we need to think about whether we need to do this here. 
   
   If we need a package as the final product, then just packaged it in the build process.
   If we use a package just to let users move it easily, let's call it `make package`, but I don't think it's needed and will take the confusion with `make build`.
   
   cc @moonming @membphis any ideas?




----------------------------------------------------------------
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] [apisix-dashboard] gxthrj commented on a change in pull request #807: chore: rename makefile release-src to release

Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #807:
URL: https://github.com/apache/apisix-dashboard/pull/807#discussion_r524300851



##########
File path: Makefile
##########
@@ -91,5 +93,10 @@ endif
 
 .PHONY: release
 release:
-	tar –cf dashboard.tar ./output/*
+	tar -zcvf $(RELEASE).tgz ./output/*

Review comment:
       Ok, It seems that `make release` should not be used here.
   The `output` directory is not source code. They are the result of compilation, including manager and web.
   Maybe `make publish` is more suitable. Ref to some web projects , such as elementUI.




----------------------------------------------------------------
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] [apisix-dashboard] codecov-io edited a comment on pull request #807: chore: rename makefile release-src to release

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #807:
URL: https://github.com/apache/apisix-dashboard/pull/807#issuecomment-727555552


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/807?src=pr&el=h1) Report
   > Merging [#807](https://codecov.io/gh/apache/apisix-dashboard/pull/807?src=pr&el=desc) (1629fda) into [v2.0](https://codecov.io/gh/apache/apisix-dashboard/commit/6dae80b5ca6edc71cfdc5dc08bd70af8ccac4a36?el=desc) (6dae80b) will **decrease** coverage by `0.07%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/807/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/807?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##             v2.0     #807      +/-   ##
   ==========================================
   - Coverage   42.64%   42.56%   -0.08%     
   ==========================================
     Files          18       18              
     Lines        1257     1257              
   ==========================================
   - Hits          536      535       -1     
   - Misses        630      631       +1     
     Partials       91       91              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/807?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [api/internal/core/store/store.go](https://codecov.io/gh/apache/apisix-dashboard/pull/807/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmUuZ28=) | `78.28% <0.00%> (-0.66%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/807?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/807?src=pr&el=footer). Last update [6dae80b...1629fda](https://codecov.io/gh/apache/apisix-dashboard/pull/807?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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] [apisix-dashboard] juzhiyuan commented on a change in pull request #807: chore: rename makefile release-src to release

Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #807:
URL: https://github.com/apache/apisix-dashboard/pull/807#discussion_r523747799



##########
File path: Makefile
##########
@@ -89,7 +89,7 @@ endif
 	.actions/openwhisk-utilities/scancode/scanCode.py --config .actions/ASF-Release.cfg ./
 
 
-.PHONY: release-src
-release-src:
+.PHONY: release

Review comment:
       Does release only do a package work? 
   
   cc @moonming @ShiningRush any ideas? 




----------------------------------------------------------------
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] [apisix-dashboard] gxthrj commented on a change in pull request #807: chore: rename makefile release-src to release

Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #807:
URL: https://github.com/apache/apisix-dashboard/pull/807#discussion_r525048846



##########
File path: .gitignore
##########
@@ -42,6 +42,9 @@ build
 manager-api
 
 output
+publish

Review comment:
       It is for the issue #787 
   It seems that I didn't understood the meaning of issue. 




----------------------------------------------------------------
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] [apisix-dashboard] juzhiyuan commented on a change in pull request #807: chore: rename makefile release-src to release

Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #807:
URL: https://github.com/apache/apisix-dashboard/pull/807#discussion_r523747799



##########
File path: Makefile
##########
@@ -89,7 +89,7 @@ endif
 	.actions/openwhisk-utilities/scancode/scanCode.py --config .actions/ASF-Release.cfg ./
 
 
-.PHONY: release-src
-release-src:
+.PHONY: release

Review comment:
       Does release only do a package work? 
   
   cc @moonming @ShiningRush 




----------------------------------------------------------------
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] [apisix-dashboard] codecov-io commented on pull request #807: chore: rename makefile release-src to release

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #807:
URL: https://github.com/apache/apisix-dashboard/pull/807#issuecomment-727555552


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/807?src=pr&el=h1) Report
   > Merging [#807](https://codecov.io/gh/apache/apisix-dashboard/pull/807?src=pr&el=desc) (9824c23) into [v2.0](https://codecov.io/gh/apache/apisix-dashboard/commit/6dae80b5ca6edc71cfdc5dc08bd70af8ccac4a36?el=desc) (6dae80b) will **not change** coverage.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/807/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/807?src=pr&el=tree)
   
   ```diff
   @@           Coverage Diff           @@
   ##             v2.0     #807   +/-   ##
   =======================================
     Coverage   42.64%   42.64%           
   =======================================
     Files          18       18           
     Lines        1257     1257           
   =======================================
     Hits          536      536           
     Misses        630      630           
     Partials       91       91           
   ```
   
   
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/807?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/807?src=pr&el=footer). Last update [6dae80b...9824c23](https://codecov.io/gh/apache/apisix-dashboard/pull/807?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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] [apisix-dashboard] gxthrj closed pull request #807: chore: rename makefile release-src to release

Posted by GitBox <gi...@apache.org>.
gxthrj closed pull request #807:
URL: https://github.com/apache/apisix-dashboard/pull/807


   


----------------------------------------------------------------
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] [apisix-dashboard] juzhiyuan commented on a change in pull request #807: chore: rename makefile release-src to release

Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #807:
URL: https://github.com/apache/apisix-dashboard/pull/807#discussion_r524823060



##########
File path: .gitignore
##########
@@ -42,6 +42,9 @@ build
 manager-api
 
 output
+publish

Review comment:
       We use `publish` as a folder is not a good idea, can you provide some examples of other popular projects using this name?
   
   The act of ordering this line is to package the product, and we need to think about whether we need to do this here. 
   
   If we need a package as the final product, then just packaged it in the build process.
   If we use a package just to let users move it easily, let's call it `make package`, but I don't think it's needed and will take the confusion with `make build`.
   
   cc @moonming @membphis any ideas?




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