Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docs: Updating HTTP API endpoints with CLI equivalent links #12004

Merged
merged 9 commits into from
Jan 15, 2022

Conversation

ChipV223
Copy link
Contributor

As the title suggests, this docs changes is to include a link to the CLI command from the equivalent HTTP API page

@ChipV223 ChipV223 added type/docs Documentation needs to be created/updated/clarified type/docs-cherrypick event/consul-docs-day A day focused on improving Consul's documentation. Date: Jan 10, 2022. labels Jan 10, 2022
@ChipV223 ChipV223 requested review from markan, jkirschner-hashicorp and a team January 10, 2022 21:11
@ChipV223 ChipV223 changed the title Updating HTTP API endpoints with CLI equivalent links (draft) Docs: Updating HTTP API endpoints with CLI equivalent links Jan 10, 2022
@jkirschner-hashicorp jkirschner-hashicorp added the pr/no-changelog PR does not need a corresponding .changelog entry label Jan 11, 2022
Copy link
Contributor

@jkirschner-hashicorp jkirschner-hashicorp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ChipV223 : looks good! I'm excited to have these additions in the docs.

I spotted some minor corrections that (unfortunately) affect every line, but hopefully won't take too long to fix. I also have an open question for @karl-cardenas-coding / @trujillo-adam. Let's wait for them to respond before making any changes so that you just need to make one more pass through everything.

One other thing I noticed: none of the admin partition commands are included in this change. These don't exist in my spreadsheet because I created the spreadsheet before admin partitions were released. Should we include those in this PR? Or will that need to be done separately? I posted a similar question to @msiege2 on #11984.

@@ -34,7 +34,9 @@ The table below shows this endpoint's support for
| ---------------- | ----------------- | ------------- | ------------ |
| `NO` | `none` | `none` | `acl:write` |

### Payload Fields
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for noticing and correcting this inconsistency :)

@@ -156,6 +160,8 @@ The table below shows this endpoint's support for
| ---------------- | ----------------- | ------------- | ------------ |
| `YES` | `all` | `none` | `acl:read` |

-> The corresponding CLI command is [`consul acl policy read -name=<string>`](https://www.consul.io/commands/acl/policy/read#name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice - I like the use of the flag here!

@ChipV223
Copy link
Contributor Author

@jkirschner-hashicorp : The latest commit contains the links fixes as well as the addition of "." at the end of the sentence

Copy link
Contributor

@jkirschner-hashicorp jkirschner-hashicorp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last thing to change and then we're good to go!

Admin partition stuff looks good, thanks.

@@ -34,7 +34,9 @@ The table below shows this endpoint's support for
| ---------------- | ----------------- | ------------- | ------------ |
| `NO` | `none` | `none` | `acl:write` |

### Payload Fields
-> The corresponding CLI command is [`consul acl auth-method create`](/commands/acl/auth-method/create).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After discussing with @karl-cardenas-coding and @trujillo-adam , we agree that we need to remove the info box treatment for these sentences (that gives them the blue background). Can you go through all these and remove the -> at the front?

Suggested change
-> The corresponding CLI command is [`consul acl auth-method create`](/commands/acl/auth-method/create).
The corresponding CLI command is [`consul acl auth-method create`](/commands/acl/auth-method/create).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I addressed my own feedback

@jkirschner-hashicorp jkirschner-hashicorp changed the title (draft) Docs: Updating HTTP API endpoints with CLI equivalent links Docs: Updating HTTP API endpoints with CLI equivalent links Jan 15, 2022
@jkirschner-hashicorp jkirschner-hashicorp merged commit 8a22562 into main Jan 15, 2022
@jkirschner-hashicorp jkirschner-hashicorp deleted the consul-doc-day-changes-chip branch January 15, 2022 17:40
@hc-github-team-consul-core
Copy link
Collaborator

🍒 If backport labels were added before merging, cherry-picking will start automatically.

To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/555551.

@hc-github-team-consul-core
Copy link
Collaborator

🍒✅ Cherry pick of commit 8a22562 onto stable-website succeeded!

hc-github-team-consul-core pushed a commit that referenced this pull request Jan 15, 2022
Docs: Updating HTTP API endpoints with CLI equivalent links
@hc-github-team-consul-core
Copy link
Collaborator

🍒✅ Cherry pick of commit 8a22562 onto release/1.11.x succeeded!

hc-github-team-consul-core pushed a commit that referenced this pull request Jan 15, 2022
Docs: Updating HTTP API endpoints with CLI equivalent links
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
event/consul-docs-day A day focused on improving Consul's documentation. Date: Jan 10, 2022. pr/no-changelog PR does not need a corresponding .changelog entry type/docs Documentation needs to be created/updated/clarified
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants