You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2020/06/30 23:12:05 UTC

[GitHub] [calcite] zabetak opened a new pull request #2050: Site: Remove '(for Calcite committers)' suffix from headers in section dedicated to committers

zabetak opened a new pull request #2050:
URL: https://github.com/apache/calcite/pull/2050


   


----------------------------------------------------------------
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] [calcite] rubenada commented on a change in pull request #2050: Site: Remove '(for Calcite committers)' suffix from headers in section dedicated to committers

Posted by GitBox <gi...@apache.org>.
rubenada commented on a change in pull request #2050:
URL: https://github.com/apache/calcite/pull/2050#discussion_r448327076



##########
File path: site/_docs/howto.md
##########
@@ -474,7 +474,7 @@ $ ./src/main/scripts/generate-protobuf.sh
 The following sections are of interest to Calcite committers and in
 particular release managers.
 
-## Managing Calcite repositories through GitHub (for Calcite committers)
+## Managing Calcite repositories through GitHub
 
 Committers have write access to Calcite's 
 [ASF git repositories](https://gitbox.apache.org/repos/asf#calcite) hosting

Review comment:
       I think @zabetak has a point. If we have a section called "Advanced topics for committers", then it seems redundant to have every subsection in there called "XYZ (for Calcite committers)".
   Right before that we have an "Advanced topics for developers" section and we do not have the suffix "(for developers)" on each subsection.




----------------------------------------------------------------
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] [calcite] zinking commented on pull request #2050: Site: Remove '(for Calcite committers)' suffix from headers in section dedicated to committers

Posted by GitBox <gi...@apache.org>.
zinking commented on pull request #2050:
URL: https://github.com/apache/calcite/pull/2050#issuecomment-652955372


   > Thanks everybody for your feedback. Given that there is a -1, I cannot move this forward. @zinking I don't want to start a long exchange about such a minor issue but I would like to know why do you think its not an improvement. Can you please elaborate a bit more?
   
   like you said, it's such a minor change. and I'm good with ruben's explaination now. please move forward.


----------------------------------------------------------------
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] [calcite] rubenada commented on pull request #2050: Site: Remove '(for Calcite committers)' suffix from headers in section dedicated to committers

Posted by GitBox <gi...@apache.org>.
rubenada commented on pull request #2050:
URL: https://github.com/apache/calcite/pull/2050#issuecomment-652827803


   LGTM


----------------------------------------------------------------
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] [calcite] chunweilei commented on a change in pull request #2050: Site: Remove '(for Calcite committers)' suffix from headers in section dedicated to committers

Posted by GitBox <gi...@apache.org>.
chunweilei commented on a change in pull request #2050:
URL: https://github.com/apache/calcite/pull/2050#discussion_r448728958



##########
File path: site/_docs/howto.md
##########
@@ -474,7 +474,7 @@ $ ./src/main/scripts/generate-protobuf.sh
 The following sections are of interest to Calcite committers and in
 particular release managers.
 
-## Managing Calcite repositories through GitHub (for Calcite committers)
+## Managing Calcite repositories through GitHub
 
 Committers have write access to Calcite's 
 [ASF git repositories](https://gitbox.apache.org/repos/asf#calcite) hosting

Review comment:
       > The whole section is about committers (see line 472) and the suffix is repeated in every subsection so IMHO it is redundant. It makes headers and links longer without adding new information.
   
   Makes sense. Thanks for your explanation.




----------------------------------------------------------------
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] [calcite] zabetak merged pull request #2050: Site: Remove '(for Calcite committers)' suffix from headers in section dedicated to committers

Posted by GitBox <gi...@apache.org>.
zabetak merged pull request #2050:
URL: https://github.com/apache/calcite/pull/2050


   


----------------------------------------------------------------
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] [calcite] chunweilei commented on a change in pull request #2050: Site: Remove '(for Calcite committers)' suffix from headers in section dedicated to committers

Posted by GitBox <gi...@apache.org>.
chunweilei commented on a change in pull request #2050:
URL: https://github.com/apache/calcite/pull/2050#discussion_r448156429



##########
File path: site/_docs/howto.md
##########
@@ -474,7 +474,7 @@ $ ./src/main/scripts/generate-protobuf.sh
 The following sections are of interest to Calcite committers and in
 particular release managers.
 
-## Managing Calcite repositories through GitHub (for Calcite committers)
+## Managing Calcite repositories through GitHub
 
 Committers have write access to Calcite's 
 [ASF git repositories](https://gitbox.apache.org/repos/asf#calcite) hosting

Review comment:
       Why should we remove the suffix?




----------------------------------------------------------------
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] [calcite] zabetak commented on pull request #2050: Site: Remove '(for Calcite committers)' suffix from headers in section dedicated to committers

Posted by GitBox <gi...@apache.org>.
zabetak commented on pull request #2050:
URL: https://github.com/apache/calcite/pull/2050#issuecomment-652904341


   Thanks everybody for your feedback. Given that there is a -1, I cannot move this forward. @zinking I don't want to start a long exchange about such a minor issue but I would like to know why do you think its not an improvement. Can you please elaborate a bit more? 


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