Skip to content

Vzlamal/fix installing ca with existing keys in internal token#5059

Merged
ladycfu merged 4 commits intodogtagpki:masterfrom
vzlamal:vzlamal/fix-installing-ca-with-existing-keys-in-internal-token
May 8, 2025
Merged

Vzlamal/fix installing ca with existing keys in internal token#5059
ladycfu merged 4 commits intodogtagpki:masterfrom
vzlamal:vzlamal/fix-installing-ca-with-existing-keys-in-internal-token

Conversation

@vzlamal
Copy link
Copy Markdown
Contributor

@vzlamal vzlamal commented Apr 28, 2025

Suggesting fix for adoc file that is not correctly rendered.

@sonarqubecloud
Copy link
Copy Markdown

@vzlamal
Copy link
Copy Markdown
Contributor Author

vzlamal commented Apr 30, 2025

I actually think that using built in commands is even cleaner than paring it with sed. So I updated the section.

Copy link
Copy Markdown
Contributor

@ladycfu ladycfu left a comment

Choose a reason for hiding this comment

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

replacement to use "pki-server cert-export" instead of echo and sed looks good.
Question: what's the reason for replacing [literal,subs="+quotes,verbatim"] with [literal]?

@vzlamal
Copy link
Copy Markdown
Contributor Author

vzlamal commented May 6, 2025

In the previous version, substitutions caused the sed command to be rendered partially in bold instead of preserving it as-is. This shouldn’t be an issue in the current version, but I consider this a code block that should remain unchanged so no substitutions should be applied. This approach also helps prevent similar formatting bugs in the future if the code block is modified.

@ladycfu
Copy link
Copy Markdown
Contributor

ladycfu commented May 7, 2025

@vzlamal while our writer is looking into the effect of [literal,subs="+quotes,verbatim"] v.s. [literal], how about for this patch, you put back the [literal,subs="+quotes,verbatim"] and do the same pki-server substitution for https://github.com/dogtagpki/pki/blob/master/docs/installation/ca/installing-ca-with-existing-keys-in-hsm.adoc (appears to have the same issue), and I will approve the new patch. You can then submit a 2nd patch for the [literal,subs="+quotes,verbatim"] replacement and I'll ask our writer to review and go from there. How's that?

@vzlamal
Copy link
Copy Markdown
Contributor Author

vzlamal commented May 7, 2025

Sounds good. I made the change to this PR and here is a new one #5077 for the writer.

Copy link
Copy Markdown
Contributor

@ladycfu ladycfu left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@ladycfu ladycfu merged commit 8904738 into dogtagpki:master May 8, 2025
1 of 7 checks passed
@ladycfu
Copy link
Copy Markdown
Contributor

ladycfu commented May 8, 2025

@vzlamal I just realized that community members can't merge themselves. I just merged it for you. Thanks again for the patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants