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 2021/05/03 07:36:48 UTC
[GitHub] [apisix] KishaniKandasamy opened a new pull request #4175: docs: fix broken links and ids in plugins
KishaniKandasamy opened a new pull request #4175:
URL: https://github.com/apache/apisix/pull/4175
### What this PR does / why we need it:
fix: [#325](https://github.com/apache/apisix-website/issues/325)
### Pre-submission checklist:
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [ ] Have you added corresponding test cases?
* [ ] Have you modified the corresponding document?
* [ ] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
--
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] spacewander commented on a change in pull request #4175: docs: fix broken links and ids in plugins
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4175:
URL: https://github.com/apache/apisix/pull/4175#discussion_r625542469
##########
File path: docs/en/latest/config.json
##########
@@ -162,6 +162,13 @@
"discovery/eureka"
]
},
+ {
+ "type": "category",
+ "label": "Examples",
Review comment:
And this problem should be fixed by the doc render framework itself. Adding unrelative doc to the sidebar to fix it is **a hack**.
--
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] iamayushdas edited a comment on pull request #4175: docs: fix broken links and ids in plugins
Posted by GitBox <gi...@apache.org>.
iamayushdas edited a comment on pull request #4175:
URL: https://github.com/apache/apisix/pull/4175#issuecomment-831805292
@spacewander what do you think about adding or moreover nesting example inside authentication -> hmac-auth -> example: hmac generate signature ?
@KishaniKandasamy want to know your opinion too
--
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] KishaniKandasamy commented on a change in pull request #4175: docs: fix broken links and ids in plugins
Posted by GitBox <gi...@apache.org>.
KishaniKandasamy commented on a change in pull request #4175:
URL: https://github.com/apache/apisix/pull/4175#discussion_r625503789
##########
File path: docs/en/latest/config.json
##########
@@ -162,6 +162,13 @@
"discovery/eureka"
]
},
+ {
+ "type": "category",
+ "label": "Examples",
Review comment:
then Shall I add this directly to the sidebar?I don't know how to add https://github.com/apache/apisix/blob/master/docs/en/latest/examples/plugins-hmac-auth-generate-signature.md tor.Sorry
--
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] iamayushdas commented on pull request #4175: docs: fix broken links and ids in plugins
Posted by GitBox <gi...@apache.org>.
iamayushdas commented on pull request #4175:
URL: https://github.com/apache/apisix/pull/4175#issuecomment-831275128
@guoqqi @tokers have a look to this
--
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] spacewander commented on pull request #4175: docs: fix broken links and ids in plugins
Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #4175:
URL: https://github.com/apache/apisix/pull/4175#issuecomment-831740037
@KishaniKandasamy
1. Don't change something that works well.
2. Before you submit a PR, please discuss it carefully and limit the scope. Most of your change in this PR is not relative to "broken links" or `ids`. Even without a quote, I think most people will think https://host.domain/auth/realms/foo/.well-known/uma2-configuration is just a "fake" example instead of a real link. Those links are **used as the example**. How many people in the real world have been misled by those links? What's the evidence?
--
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] KishaniKandasamy commented on a change in pull request #4175: docs: fix broken links and ids in plugins
Posted by GitBox <gi...@apache.org>.
KishaniKandasamy commented on a change in pull request #4175:
URL: https://github.com/apache/apisix/pull/4175#discussion_r625500678
##########
File path: docs/en/latest/plugins/skywalking.md
##########
@@ -101,7 +101,7 @@ plugin_attr:
skywalking:
service_name: APISIX
service_instance_name: "APISIX Instance Name"
- endpoint_addr: http://127.0.0.1:12800
+ endpoint_addr: `http://127.0.0.1:12800`
Review comment:
> Don't blindly add '``'.
OOOkay
--
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] KishaniKandasamy commented on a change in pull request #4175: docs: fix broken links and ids in plugins
Posted by GitBox <gi...@apache.org>.
KishaniKandasamy commented on a change in pull request #4175:
URL: https://github.com/apache/apisix/pull/4175#discussion_r625550360
##########
File path: docs/en/latest/plugins/authz-keycloak.md
##########
@@ -42,9 +42,9 @@ For more information on Keycloak, refer to [Keycloak Authorization Docs](https:/
| Name | Type | Requirement | Default | Valid | Description |
| ------------------------------ | ------------- | ----------- | --------------------------------------------- | ------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| discovery | string | optional | | https://host.domain/auth/realms/foo/.well-known/uma2-configuration | URL to discovery document for Keycloak Authorization Services. |
-| token_endpoint | string | optional | | https://host.domain/auth/realms/foo/protocol/openid-connect/token | A OAuth2-compliant Token Endpoint that supports the `urn:ietf:params:oauth:grant-type:uma-ticket` grant type. Overrides value from discovery, if given. |
-| resource_registration_endpoint | string | optional | | https://host.domain/auth/realms/foo/authz/protection/resource_set | A Keycloak Protection API-compliant resource registration endpoint. Overrides value from discovery, if given. |
+| discovery | string | optional | | `https://host.domain/auth/realms/foo/.well-known/uma2-configuration` | URL to discovery document for Keycloak Authorization Services. |
Review comment:
I thought ,If a phrase doesn't direct to any page ,we could better go with `` rather than explicitly showing it as a link,,something like
It would be more helpful to me if you @spacewander explain me why you used (``) `http://127.0.0.1:9080/apisix/dashboard/` in https://apisix.apache.org/docs/apisix/plugins/limit-req#how-to-enable-on-the-route-or-serivce. As I really don't know how to explain this, I would like to request @juzhiyuan too to review this PR ;)
--
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] KishaniKandasamy commented on pull request #4175: docs: fix broken links and ids in plugins
Posted by GitBox <gi...@apache.org>.
KishaniKandasamy commented on pull request #4175:
URL: https://github.com/apache/apisix/pull/4175#issuecomment-832105117
> @iamayushdas @KishaniKandasamy
> Have you read my review?
>
> > Not all pages need a sidebar. They can go back from the browser.
>
> > And this problem should be fixed by the doc render framework itself. Adding unrelative doc to the sidebar to fix it is a hack.
>
> I won't want to review anymore if your guys keep ignoring my feedback.
>
> And such a change should not have such a title called 'docs: fix broken links and ids in plugins'!
>
> Please open another PR with **correct** name and **suitable** change!
>
> I am quite annoyed by you today.
I am really Sorry for the hassle! I will try to open another PR later
--
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] spacewander commented on a change in pull request #4175: docs: fix broken links and ids in plugins
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4175:
URL: https://github.com/apache/apisix/pull/4175#discussion_r625512291
##########
File path: docs/en/latest/plugins/authz-keycloak.md
##########
@@ -42,9 +42,9 @@ For more information on Keycloak, refer to [Keycloak Authorization Docs](https:/
| Name | Type | Requirement | Default | Valid | Description |
| ------------------------------ | ------------- | ----------- | --------------------------------------------- | ------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| discovery | string | optional | | https://host.domain/auth/realms/foo/.well-known/uma2-configuration | URL to discovery document for Keycloak Authorization Services. |
-| token_endpoint | string | optional | | https://host.domain/auth/realms/foo/protocol/openid-connect/token | A OAuth2-compliant Token Endpoint that supports the `urn:ietf:params:oauth:grant-type:uma-ticket` grant type. Overrides value from discovery, if given. |
-| resource_registration_endpoint | string | optional | | https://host.domain/auth/realms/foo/authz/protection/resource_set | A Keycloak Protection API-compliant resource registration endpoint. Overrides value from discovery, if given. |
+| discovery | string | optional | | `https://host.domain/auth/realms/foo/.well-known/uma2-configuration` | URL to discovery document for Keycloak Authorization Services. |
Review comment:
I can't understand what you mean. Does adding '``' make things different?
--
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] spacewander commented on a change in pull request #4175: docs: fix broken links and ids in plugins
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4175:
URL: https://github.com/apache/apisix/pull/4175#discussion_r625541979
##########
File path: docs/en/latest/config.json
##########
@@ -162,6 +162,13 @@
"discovery/eureka"
]
},
+ {
+ "type": "category",
+ "label": "Examples",
Review comment:
Not all pages need a sidebar. They can go back from the browser.
--
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] KishaniKandasamy commented on pull request #4175: docs: fix broken links and ids in plugins
Posted by GitBox <gi...@apache.org>.
KishaniKandasamy commented on pull request #4175:
URL: https://github.com/apache/apisix/pull/4175#issuecomment-831806244
> @spacewander what do you think about adding or moreover nesting example inside authentication -> hmac-auth -> example: hmac generate signature ?
> @KishaniKandasamy want to know your opinion too
Yes Ayush That would be better IMO
--
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] spacewander commented on a change in pull request #4175: docs: fix broken links and ids in plugins
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4175:
URL: https://github.com/apache/apisix/pull/4175#discussion_r625455517
##########
File path: docs/en/latest/config.json
##########
@@ -162,6 +162,13 @@
"discovery/eureka"
]
},
+ {
+ "type": "category",
+ "label": "Examples",
Review comment:
This section is not part of the category.
##########
File path: docs/en/latest/plugins/skywalking.md
##########
@@ -101,7 +101,7 @@ plugin_attr:
skywalking:
service_name: APISIX
service_instance_name: "APISIX Instance Name"
- endpoint_addr: http://127.0.0.1:12800
+ endpoint_addr: `http://127.0.0.1:12800`
Review comment:
Don't blindly add '``'.
##########
File path: docs/en/latest/plugins/authz-keycloak.md
##########
@@ -42,9 +42,9 @@ For more information on Keycloak, refer to [Keycloak Authorization Docs](https:/
| Name | Type | Requirement | Default | Valid | Description |
| ------------------------------ | ------------- | ----------- | --------------------------------------------- | ------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| discovery | string | optional | | https://host.domain/auth/realms/foo/.well-known/uma2-configuration | URL to discovery document for Keycloak Authorization Services. |
-| token_endpoint | string | optional | | https://host.domain/auth/realms/foo/protocol/openid-connect/token | A OAuth2-compliant Token Endpoint that supports the `urn:ietf:params:oauth:grant-type:uma-ticket` grant type. Overrides value from discovery, if given. |
-| resource_registration_endpoint | string | optional | | https://host.domain/auth/realms/foo/authz/protection/resource_set | A Keycloak Protection API-compliant resource registration endpoint. Overrides value from discovery, if given. |
+| discovery | string | optional | | `https://host.domain/auth/realms/foo/.well-known/uma2-configuration` | URL to discovery document for Keycloak Authorization Services. |
Review comment:
Why add '``' surround the link?
--
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] KishaniKandasamy commented on a change in pull request #4175: docs: fix broken links and ids in plugins
Posted by GitBox <gi...@apache.org>.
KishaniKandasamy commented on a change in pull request #4175:
URL: https://github.com/apache/apisix/pull/4175#discussion_r625540565
##########
File path: docs/en/latest/config.json
##########
@@ -162,6 +162,13 @@
"discovery/eureka"
]
},
+ {
+ "type": "category",
+ "label": "Examples",
Review comment:
So when visitors see this page https://apisix.apache.org/docs/apisix/examples/plugins-hmac-auth-generate-signature, how could they see other menus?
--
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] spacewander commented on a change in pull request #4175: docs: fix broken links and ids in plugins
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4175:
URL: https://github.com/apache/apisix/pull/4175#discussion_r625560772
##########
File path: docs/en/latest/plugins/authz-keycloak.md
##########
@@ -42,9 +42,9 @@ For more information on Keycloak, refer to [Keycloak Authorization Docs](https:/
| Name | Type | Requirement | Default | Valid | Description |
| ------------------------------ | ------------- | ----------- | --------------------------------------------- | ------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| discovery | string | optional | | https://host.domain/auth/realms/foo/.well-known/uma2-configuration | URL to discovery document for Keycloak Authorization Services. |
-| token_endpoint | string | optional | | https://host.domain/auth/realms/foo/protocol/openid-connect/token | A OAuth2-compliant Token Endpoint that supports the `urn:ietf:params:oauth:grant-type:uma-ticket` grant type. Overrides value from discovery, if given. |
-| resource_registration_endpoint | string | optional | | https://host.domain/auth/realms/foo/authz/protection/resource_set | A Keycloak Protection API-compliant resource registration endpoint. Overrides value from discovery, if given. |
+| discovery | string | optional | | `https://host.domain/auth/realms/foo/.well-known/uma2-configuration` | URL to discovery document for Keycloak Authorization Services. |
Review comment:
> It would be more helpful to me if you @spacewander explain me why you used (``) http://127.0.0.1:9080/apisix/dashboard/ in apisix.apache.org/docs/apisix/plugins/limit-req#how-to-enable-on-the-route-or-serivce.
You should not ask me. If you know how to use `git blame`, you can see **every lines in this page is not written by me**. How can I answer you?
--
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] iamayushdas edited a comment on pull request #4175: docs: fix broken links and ids in plugins
Posted by GitBox <gi...@apache.org>.
iamayushdas edited a comment on pull request #4175:
URL: https://github.com/apache/apisix/pull/4175#issuecomment-831686349
@KishaniKandasamy what do you think about adding or nesting it into authentication -> hmac-auth -> Signature generate example, or
you can add a section of example as a type of doc inside authentication,
do you agree?
--
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] iamayushdas commented on pull request #4175: docs: fix broken links and ids in plugins
Posted by GitBox <gi...@apache.org>.
iamayushdas commented on pull request #4175:
URL: https://github.com/apache/apisix/pull/4175#issuecomment-831805292
@spacewander what do you think about adding or moreover nesting example inside authentication -> hmac-auth -> example: hmac generate signature ?
--
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] iamayushdas removed a comment on pull request #4175: docs: fix broken links and ids in plugins
Posted by GitBox <gi...@apache.org>.
iamayushdas removed a comment on pull request #4175:
URL: https://github.com/apache/apisix/pull/4175#issuecomment-831686349
@KishaniKandasamy what do you think about adding or nesting it into authentication -> hmac-auth -> Signature generate example, or
you can add a section of example as a type of doc inside authentication,
do you agree?
--
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] spacewander closed pull request #4175: docs: fix broken links and ids in plugins
Posted by GitBox <gi...@apache.org>.
spacewander closed pull request #4175:
URL: https://github.com/apache/apisix/pull/4175
--
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] KishaniKandasamy commented on a change in pull request #4175: docs: fix broken links and ids in plugins
Posted by GitBox <gi...@apache.org>.
KishaniKandasamy commented on a change in pull request #4175:
URL: https://github.com/apache/apisix/pull/4175#discussion_r625500422
##########
File path: docs/en/latest/plugins/authz-keycloak.md
##########
@@ -42,9 +42,9 @@ For more information on Keycloak, refer to [Keycloak Authorization Docs](https:/
| Name | Type | Requirement | Default | Valid | Description |
| ------------------------------ | ------------- | ----------- | --------------------------------------------- | ------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| discovery | string | optional | | https://host.domain/auth/realms/foo/.well-known/uma2-configuration | URL to discovery document for Keycloak Authorization Services. |
-| token_endpoint | string | optional | | https://host.domain/auth/realms/foo/protocol/openid-connect/token | A OAuth2-compliant Token Endpoint that supports the `urn:ietf:params:oauth:grant-type:uma-ticket` grant type. Overrides value from discovery, if given. |
-| resource_registration_endpoint | string | optional | | https://host.domain/auth/realms/foo/authz/protection/resource_set | A Keycloak Protection API-compliant resource registration endpoint. Overrides value from discovery, if given. |
+| discovery | string | optional | | `https://host.domain/auth/realms/foo/.well-known/uma2-configuration` | URL to discovery document for Keycloak Authorization Services. |
Review comment:
I thought they are just **valid url format** and it seems they don't direct to anywhere.
--
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] spacewander commented on a change in pull request #4175: docs: fix broken links and ids in plugins
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4175:
URL: https://github.com/apache/apisix/pull/4175#discussion_r625513277
##########
File path: docs/en/latest/config.json
##########
@@ -162,6 +162,13 @@
"discovery/eureka"
]
},
+ {
+ "type": "category",
+ "label": "Examples",
Review comment:
Don't add it to the sidebar. It doesn't belong to that.
Please revert the change.
Please explain yourself clearly when submitting the change.
I don't even know why you want to add it to the sidebar.
--
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] spacewander commented on pull request #4175: docs: fix broken links and ids in plugins
Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #4175:
URL: https://github.com/apache/apisix/pull/4175#issuecomment-831870516
@iamayushdas @KishaniKandasamy
Have you read my review?
> Not all pages need a sidebar. They can go back from the browser.
> And this problem should be fixed by the doc render framework itself. Adding unrelative doc to the sidebar to fix it is a hack.
I won't want to review anymore if your guys keep ignoring my feedback.
And such a change should not have such a title called 'docs: fix broken links and ids in plugins'!
Please open another PR with **correct** name and **suitable** change!
I am quite annoyed by you today.
--
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] iamayushdas edited a comment on pull request #4175: docs: fix broken links and ids in plugins
Posted by GitBox <gi...@apache.org>.
iamayushdas edited a comment on pull request #4175:
URL: https://github.com/apache/apisix/pull/4175#issuecomment-831275128
@Yiyiyimu @tokers have a look to this please
--
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] iamayushdas commented on pull request #4175: docs: fix broken links and ids in plugins
Posted by GitBox <gi...@apache.org>.
iamayushdas commented on pull request #4175:
URL: https://github.com/apache/apisix/pull/4175#issuecomment-831686349
@KishaniKandasamy what do you think about adding or nesting it into authentication -> hmac-auth -> Signature generate example
--
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] spacewander commented on pull request #4175: docs: fix broken links and ids in plugins
Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #4175:
URL: https://github.com/apache/apisix/pull/4175#issuecomment-831628212
Would you put similar changes into one PR instead of submitting them one by one? 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org