Skip to content

Format the issuers table#1383

Merged
jetstack-bot merged 2 commits intocert-manager:masterfrom
wallrj:format-issuers-table
Jan 11, 2024
Merged

Format the issuers table#1383
jetstack-bot merged 2 commits intocert-manager:masterfrom
wallrj:format-issuers-table

Conversation

@wallrj
Copy link
Copy Markdown
Member

@wallrj wallrj commented Jan 11, 2024

While reviewing #1382 I wanted to help the author move their new issuer to the right place in the table,
but the markdown table is currently unaligned and difficult to read in the GitHub preview,
so here I'm trying to fix that problem first.

image

vs

image

@jetstack-bot jetstack-bot added dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 11, 2024
@wallrj wallrj force-pushed the format-issuers-table branch from 9adc633 to 6685981 Compare January 11, 2024 09:54
@netlify
Copy link
Copy Markdown

netlify bot commented Jan 11, 2024

Deploy Preview for cert-manager-website ready!

Name Link
🔨 Latest commit 9adc633
🔍 Latest deploy log https://app.netlify.com/sites/cert-manager-website/deploys/659fba670ff2d100070ad507
😎 Deploy Preview https://deploy-preview-1383--cert-manager-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link
Copy Markdown

netlify bot commented Jan 11, 2024

Deploy Preview for cert-manager-website ready!

Name Link
🔨 Latest commit 3c913f6
🔍 Latest deploy log https://app.netlify.com/sites/cert-manager-website/deploys/659fc4b80922960008ed3add
😎 Deploy Preview https://deploy-preview-1383--cert-manager-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@wallrj wallrj force-pushed the format-issuers-table branch 2 times, most recently from 748f3df to 35567e8 Compare January 11, 2024 10:30
Signed-off-by: Richard Wall <richard.wall@venafi.com>
@wallrj wallrj force-pushed the format-issuers-table branch from 35567e8 to 70761de Compare January 11, 2024 10:32
Signed-off-by: Richard Wall <richard.wall@venafi.com>

[config:venafi-enhanced-issuer]: https://docs.venafi.cloud/vaas/k8s-components/t-vei-install/
[config:acme-issuer]: ./acme.md
[config:acme-issuer]: ./acme/README.md
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This broken link was picked up by the link checker.
Not sure why it hadn't highlighted it in earlier PRs.

[ca:cfssl-issuer]: https://github.com/cloudflare/cfssl
[ca:freeipa-issuer]: https://www.freeipa.org
[ca:kms-issuer]: https://aws.amazon.com/kms/
[ca:origin-ca-issuer]: https://developers.cloudflare.com/ssl/origin-configuration/origin-ca
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I moved all the links to the CA documentation out of the table,
to make the table easier to read and for consistency with the other links in the table which are all aliased.

@wallrj wallrj requested a review from maelvls January 11, 2024 10:43
| 🥉 | freeipa-issuer | [📄][config:freeipa-issuer] | [FreeIPA][ca:freeipa-issuer] | - | [❌][release:freeipa-issuer] | ✔️ |
| 🥉 | kms-issuer | [📄][config:kms-issuer] | [AWS KMS][ca:kms-issuer] | - | [❌][release:kms-issuer] | ✔️ |
| 🥉 | origin-ca-issuer | [📄][config:origin-ca-issuer] | [Cloudflare Origin CA][ca:origin-ca-issuer] | - | [❌][release:origin-ca-issuer] | ✔️ |

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This and the earlier blank line between the markdown table and the surrounding html tags are what fix the table rendering in GitHub file preview.

Copy link
Copy Markdown
Member

@maelvls maelvls Jan 11, 2024

Choose a reason for hiding this comment

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

Thanks for the context. I have myself hit this issue of broken table due to an html tag too close to the bottom of the table multiple times.

@maelvls
Copy link
Copy Markdown
Member

maelvls commented Jan 11, 2024

/lgtm
/approve

@jetstack-bot jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 11, 2024
@jetstack-bot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: maelvls

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jetstack-bot jetstack-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 11, 2024
@jetstack-bot jetstack-bot merged commit aba4393 into cert-manager:master Jan 11, 2024
@wallrj wallrj deleted the format-issuers-table branch January 11, 2024 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants