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

Switch to Kramdown engine #986

Merged
merged 1 commit into from Oct 26, 2018

Conversation

Projects
None yet
3 participants
@vividh
Contributor

vividh commented Oct 24, 2018

Summary

Switch to Jekyll's default markdown engine Kramdown.

Kramdown fixes the TOC issue present in articles with duplicate headings, such as https://docs.konghq.com/0.14.x/admin-api/.

Additionally, before migrating from existing engine Redcarpet, the following points need to be verified to make sure nothing breaks in the docs:

  • No autolink usages in the articles
  • No superscript usages in the articles
  • No markdown present inside html blocks. If so, add markdown="1" attribute to these html tags
  • Visual comparison (non-exhaustive) between redcarpet and kramdown versions does not reveal any differences in articles (especially with ubiquitous components like code snippets, tables etc.)

Found no instances of the above in the docs. Someone more familiar with the contents of the docs could verify.

Full changelog

  • Update Jekyll config to use Kramdown instead of Redcarpet

Issues resolved

Fix #933, #942

Checklist:

@allejo

This comment has been minimized.

Contributor

allejo commented Oct 24, 2018

This switch to kramdown should also fix #942. Could you verify this? See #920 (comment)

@vividh

This comment has been minimized.

Contributor

vividh commented Oct 25, 2018

Sure @allejo. Just checked the proxy article mentioned in the comment. Here is how the generated TOC looks with kramdown:

screen shot 2018-10-25 at 9 51 46 am

Looks right to me. Although I'm confused how y'all handled the H5,H6 heading issue with redcarpet, since https://docs.konghq.com/0.14.x/proxy/#table-of-contents isn't broken either. Is there any change I need to revert first before verifying this?

@vividh vividh force-pushed the vividh:chore/jekyll-markdown-upgrade branch from a70841e to 994ca5a Oct 25, 2018

@coopr

This comment has been minimized.

Member

coopr commented Oct 25, 2018

Looks right to me. Although I'm confused how y'all handled the H5,H6 heading issue with redcarpet, since https://docs.konghq.com/0.14.x/proxy/#table-of-contents isn't broken either. Is there any change I need to revert first before verifying this?

Whoops, I fixed that page already! Have a look at https://docs.konghq.com/0.13.x/proxy/ for the problem not-yet-fixed :)

@vividh

This comment has been minimized.

Contributor

vividh commented Oct 26, 2018

Oh cool! Kramdown fixed this as well!
Updating PR to add #942 to the issues this fixes.

screen shot 2018-10-26 at 9 17 42 am

@coopr

This comment has been minimized.

Member

coopr commented Oct 26, 2018

Nice work @vividh thanks so much!

@coopr coopr merged commit e690f34 into Kong:master Oct 26, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@vividh

This comment has been minimized.

Contributor

vividh commented Oct 26, 2018

Thanks! Happy to help out!

@coopr

This comment has been minimized.

Member

coopr commented Oct 31, 2018

@vividh does app/_plugins/markdown.rb need to be updated too?

@coopr

This comment has been minimized.

Member

coopr commented Oct 31, 2018

Also @vividh , on line #169 of app/_hub/_init/my-extension/index.md should we specify "kramdown flavor" instead?

@vividh

This comment has been minimized.

Contributor

vividh commented Nov 1, 2018

@vividh does app/_plugins/markdown.rb need to be updated too?

We probably should, for consistency. Although I don't see any usages of the markdown plugin in the docs.

Also @vividh , on line #169 of app/_hub/_init/my-extension/index.md should we specify "kramdown flavor" instead?

Yeah sure. We could also remove the redcarpet dependency from Gemfile. I'll send a PR for these updates.

coopr added a commit that referenced this pull request Nov 5, 2018

coopr added a commit that referenced this pull request Nov 5, 2018

coopr added a commit that referenced this pull request Nov 5, 2018

Fix/redirect regression (#1025)
* Revert "chore(jekyll-markdown) Switch to Kramdown (#986)"

This reverts commit e690f34.

* Revert "Revert "chore(jekyll-markdown) Switch to Kramdown (#986)""

This reverts commit 0056bd3.

* Revert "docs(*) remove dead code (#1016)"

This reverts commit 3ea99b3.

kikito added a commit that referenced this pull request Dec 4, 2018

kikito added a commit that referenced this pull request Dec 4, 2018

Fix/redirect regression (#1025)
* Revert "chore(jekyll-markdown) Switch to Kramdown (#986)"

This reverts commit e690f34.

* Revert "Revert "chore(jekyll-markdown) Switch to Kramdown (#986)""

This reverts commit 0056bd3.

* Revert "docs(*) remove dead code (#1016)"

This reverts commit 3ea99b3.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment