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

Use Acsiidoc as README format #777

Merged
merged 11 commits into from
Feb 6, 2024
Merged

Use Acsiidoc as README format #777

merged 11 commits into from
Feb 6, 2024

Conversation

bdemers
Copy link
Member

@bdemers bdemers commented Apr 20, 2023

To simplify the release process, we can migrate the README to adoc and use variables for things like the project version.

This should simplify release automation in the future.

NOTE: This PR should NOT be merged, but instead should be re-applied after JWE work is complete

Steps:

Install kramdown:

gem which kramdown-asciidoc

Convert README:

kramdoc README.md
rm README.md
git add README.adoc

Add project version variable, and update code blocks:
See b0f106a

NOTE: Code blocks are not filtered/interpolated by default, so the subs="+attributes" directive must be added

TODO:

  • Confirm all anchors are present
  • Update Note/Warning blocks
  • Use adoc Table of Contents
  • Visually compare with README.md (side-by-side)

@bdemers
Copy link
Member Author

bdemers commented Apr 20, 2023

NOTE: There are other features we can get from adoc too, like automatic generation of the table of contents. That is not part of this PR.
I also didn't verify the results were the same as the markdown version. This PR is mostly a POC to check what is possible.

Base automatically changed from jwe to master May 18, 2023 22:21
@lhazlewood
Copy link
Contributor

@bdemers wanna rebase now?

@bdemers bdemers changed the title Draft: Use Acsiidoc as README format Use Acsiidoc as README format Sep 18, 2023
@bdemers
Copy link
Member Author

bdemers commented Sep 18, 2023

NOTE: subscript doesn't render correctly, I'll continue to look into that. If you see any other issues let me know

@bdemers bdemers marked this pull request as ready for review September 26, 2023 18:23
@bdemers
Copy link
Member Author

bdemers commented Feb 3, 2024

Rebased, (force pushed)

@bdemers bdemers marked this pull request as draft February 3, 2024 01:42
@bdemers bdemers marked this pull request as ready for review February 6, 2024 01:48
Copy link
Contributor

@lhazlewood lhazlewood left a comment

Choose a reason for hiding this comment

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

2 questions:

  1. Is this approach 'worth it' to you - i.e. do you feel it provides both a better authoring mechanism as well as reader/viewer experience?
  2. Will existing external links to our documentation still work correctly? I just thought if something references https://github.com/jwkt/jjwt/README.md#some-anchor, everything like that would break, no?


<a name="overview"></a>
## What is a JSON Web Token?
+++<a name="overview">++++++</a>+++
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean that the anchor is still valid/retained? I'm not yet familiar with this type of adoc syntax.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, both the named and generated anchors should be the same

@bdemers
Copy link
Member Author

bdemers commented Feb 6, 2024

  1. Is this approach 'worth it' to you - i.e. do you feel it provides both a better authoring mechanism as well as reader/viewer experience?

IMHO, it provides a better author experience, and it allows the version to be set with a property [in the header] (and could be automated at release times).

  1. Will existing external links to our documentation still work correctly? I just thought if something references https://github.com/jwkt/jjwt/README.md#some-anchor, everything like that would break, no?

Yes, I made a pass through last week, running something like this (browser console), in both GitHub rendered versions (and diffed them):

for (const a of document.anchors) {
    if (a.name.startsWith('user-content-')) {
        console.log(a.name.substring(13, a.name.length))
    }
}

It seems GitHub prefixes the named anchors with user-content- (I assume to avoid conflicts).
However, using the shortened fragment, e.g. README.adoc#some-anchor works too. (likely some JS to handle the URL fragments 🤷),

Copy link
Contributor

@lhazlewood lhazlewood left a comment

Choose a reason for hiding this comment

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

Ok, thanks for the confirmation! Feel free to merge - I wouldn't want you to have to go through another big merge conflict again in the future. If we miss any small details now, we can always adjust them later.

@bdemers bdemers merged commit 2694861 into master Feb 6, 2024
24 checks passed
@bdemers bdemers deleted the readme-adoc-idea branch February 6, 2024 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants