You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2021/06/23 20:01:20 UTC

[GitHub] [trafficserver] mlibbey opened a new pull request #7986: Docs: Fix pre-formatting for ratelimit plugin

mlibbey opened a new pull request #7986:
URL: https://github.com/apache/trafficserver/pull/7986


   New formatting:
   <img width="716" alt="Screen Shot 2021-06-23 at 1 00 52 PM" src="https://user-images.githubusercontent.com/387053/123160428-0d7a6100-d423-11eb-9f80-78e87a4f7213.png">
   


-- 
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] [trafficserver] SolidWallOfCode commented on a change in pull request #7986: Docs: Fix pre-formatting for ratelimit plugin

Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on a change in pull request #7986:
URL: https://github.com/apache/trafficserver/pull/7986#discussion_r660175085



##########
File path: doc/admin-guide/plugins/rate_limit.en.rst
##########
@@ -84,26 +84,34 @@ This example shows a simple rate limiting of `128` concurrently active client
 transactions, with a maximum queue size of `256`. The default of HTTP status
 code `429` is used when queue is full.
 
+::

Review comment:
       I think you could do
   ```
   when queue is full. ::
   ```
   instead of putting it on a separate line. See [here](https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#literal-blocks).




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

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] bneradt commented on a change in pull request #7986: Docs: Fix pre-formatting for ratelimit plugin

Posted by GitBox <gi...@apache.org>.
bneradt commented on a change in pull request #7986:
URL: https://github.com/apache/trafficserver/pull/7986#discussion_r661616638



##########
File path: doc/admin-guide/plugins/rate_limit.en.rst
##########
@@ -84,26 +84,34 @@ This example shows a simple rate limiting of `128` concurrently active client
 transactions, with a maximum queue size of `256`. The default of HTTP status
 code `429` is used when queue is full.
 
+::

Review comment:
       Sounds good. Thanks!




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

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] mlibbey commented on a change in pull request #7986: Docs: Fix pre-formatting for ratelimit plugin

Posted by GitBox <gi...@apache.org>.
mlibbey commented on a change in pull request #7986:
URL: https://github.com/apache/trafficserver/pull/7986#discussion_r660207705



##########
File path: doc/admin-guide/plugins/rate_limit.en.rst
##########
@@ -84,26 +84,34 @@ This example shows a simple rate limiting of `128` concurrently active client
 transactions, with a maximum queue size of `256`. The default of HTTP status
 code `429` is used when queue is full.
 
+::

Review comment:
       Indeed that works and was my original version. Then @bneradt said
   ```
   Good improvement.
   
   As a small tweak, can we put the :: on a line by itself? That seems more common:
   
   https://raw.githubusercontent.com/apache/trafficserver/master/doc/admin-guide/configuration/transparent-proxy/router-inline.en.rst
   https://raw.githubusercontent.com/apache/trafficserver/master/doc/developer-guide/debugging/debug-builds.en.rst
   To name a couple examples. Let me know if you had a reason to put the :: at the end of the previous line that I'm not aware of.```
   
    So I changed it to this version.




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

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] zwoop commented on pull request #7986: Docs: Fix pre-formatting for ratelimit plugin

Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #7986:
URL: https://github.com/apache/trafficserver/pull/7986#issuecomment-892214119


   Since I'm picking all this to 9.1.0, this one has to go in as well for 9.1.0.


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

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] bneradt commented on a change in pull request #7986: Docs: Fix pre-formatting for ratelimit plugin

Posted by GitBox <gi...@apache.org>.
bneradt commented on a change in pull request #7986:
URL: https://github.com/apache/trafficserver/pull/7986#discussion_r660708008



##########
File path: doc/admin-guide/plugins/rate_limit.en.rst
##########
@@ -84,26 +84,34 @@ This example shows a simple rate limiting of `128` concurrently active client
 transactions, with a maximum queue size of `256`. The default of HTTP status
 code `429` is used when queue is full.
 
+::

Review comment:
       Either way is fine with me. In the ATS docs I've worked with the `::` has been on a separate line, so I got used to that and thought that was our standard. If @SolidWallOfCode prefers to keep it at the end of the previous, then I'm fine with that. I notice we do both in our code.
   
   Thanks for your patience with this @mlibbey.




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

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] mlibbey commented on a change in pull request #7986: Docs: Fix pre-formatting for ratelimit plugin

Posted by GitBox <gi...@apache.org>.
mlibbey commented on a change in pull request #7986:
URL: https://github.com/apache/trafficserver/pull/7986#discussion_r661612196



##########
File path: doc/admin-guide/plugins/rate_limit.en.rst
##########
@@ -84,26 +84,34 @@ This example shows a simple rate limiting of `128` concurrently active client
 transactions, with a maximum queue size of `256`. The default of HTTP status
 code `429` is used when queue is full.
 
+::

Review comment:
       I suppose when Jon wrote
   https://docs.trafficserver.apache.org/en/9.0.x/_sources/developer-guide/documentation/conventions.en.rst.txt
   he used :: on the initial lines throughout. and in 
   https://docs.trafficserver.apache.org/en/9.0.x/developer-guide/documentation/conventions.en.html#block-content
   the text has examples like that `.. code::`; `.. sidebar::`, etc.
   
   So, thinking we should go the same line route. If you agree Brian, will change back.




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

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] mlibbey commented on a change in pull request #7986: Docs: Fix pre-formatting for ratelimit plugin

Posted by GitBox <gi...@apache.org>.
mlibbey commented on a change in pull request #7986:
URL: https://github.com/apache/trafficserver/pull/7986#discussion_r660207705



##########
File path: doc/admin-guide/plugins/rate_limit.en.rst
##########
@@ -84,26 +84,34 @@ This example shows a simple rate limiting of `128` concurrently active client
 transactions, with a maximum queue size of `256`. The default of HTTP status
 code `429` is used when queue is full.
 
+::

Review comment:
       Indeed that works and was my original version. Then @bneradt said
   ```
   Good improvement.
   
   As a small tweak, can we put the :: on a line by itself? That seems more common:
   
   https://raw.githubusercontent.com/apache/trafficserver/master/doc/admin-guide/configuration/transparent-proxy/router-inline.en.rst
   https://raw.githubusercontent.com/apache/trafficserver/master/doc/developer-guide/debugging/debug-builds.en.rst
   To name a couple examples. Let me know if you had a reason to put the :: at the end of the previous line that I'm not aware of.```
   
    So I changed it to this version.




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

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] bneradt commented on a change in pull request #7986: Docs: Fix pre-formatting for ratelimit plugin

Posted by GitBox <gi...@apache.org>.
bneradt commented on a change in pull request #7986:
URL: https://github.com/apache/trafficserver/pull/7986#discussion_r657448908



##########
File path: doc/admin-guide/plugins/rate_limit.en.rst
##########
@@ -82,27 +82,27 @@ Examples
 
 This example shows a simple rate limiting of `128` concurrently active client
 transactions, with a maximum queue size of `256`. The default of HTTP status
-code `429` is used when queue is full.
+code `429` is used when queue is full. ::

Review comment:
       Good improvement. 
   
   As a small tweak, can we put the `::` on a line by itself? That seems more common:
   
   - https://raw.githubusercontent.com/apache/trafficserver/master/doc/admin-guide/configuration/transparent-proxy/router-inline.en.rst
   - https://raw.githubusercontent.com/apache/trafficserver/master/doc/developer-guide/debugging/debug-builds.en.rst
   
   To name a couple examples. Let me know if you had a reason to put the `::` at the end of the previous line that I'm not aware of.




-- 
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] [trafficserver] SolidWallOfCode commented on a change in pull request #7986: Docs: Fix pre-formatting for ratelimit plugin

Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on a change in pull request #7986:
URL: https://github.com/apache/trafficserver/pull/7986#discussion_r660175085



##########
File path: doc/admin-guide/plugins/rate_limit.en.rst
##########
@@ -84,26 +84,34 @@ This example shows a simple rate limiting of `128` concurrently active client
 transactions, with a maximum queue size of `256`. The default of HTTP status
 code `429` is used when queue is full.
 
+::

Review comment:
       I think you could do
   ```
   when queue is full. ::
   ```
   instead of putting it on a separate line. See [here](https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#literal-blocks).




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

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] mlibbey merged pull request #7986: Docs: Fix pre-formatting for ratelimit plugin

Posted by GitBox <gi...@apache.org>.
mlibbey merged pull request #7986:
URL: https://github.com/apache/trafficserver/pull/7986


   


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

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] mlibbey commented on a change in pull request #7986: Docs: Fix pre-formatting for ratelimit plugin

Posted by GitBox <gi...@apache.org>.
mlibbey commented on a change in pull request #7986:
URL: https://github.com/apache/trafficserver/pull/7986#discussion_r658849232



##########
File path: doc/admin-guide/plugins/rate_limit.en.rst
##########
@@ -82,27 +82,27 @@ Examples
 
 This example shows a simple rate limiting of `128` concurrently active client
 transactions, with a maximum queue size of `256`. The default of HTTP status
-code `429` is used when queue is full.
+code `429` is used when queue is full. ::

Review comment:
       Done. Thanks!




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

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org