Skip to content

feat: enable support for Ruby 4.0 and new commonmarker 2.x (fixes #61) - #62

Open
galuszkak wants to merge 1 commit into
jekyll:masterfrom
galuszkak:feat/ruby-4.0
Open

feat: enable support for Ruby 4.0 and new commonmarker 2.x (fixes #61)#62
galuszkak wants to merge 1 commit into
jekyll:masterfrom
galuszkak:feat/ruby-4.0

Conversation

@galuszkak

Copy link
Copy Markdown

hey,

This is my first contribution using Ruby - so I kindly ask for through review. I'm not a rubist by a trade so my code can feel sometimes probably more pythonic / golangish.

I tested this locally with Ruby 3.2, 3.4 and 4.0.1.

Mostly I wrestle here with question should options and configuration changed to new format from commonmarker 2.x - and I did deliberately changed this from 0.20+. This maybe not best idea - but I thought it would be expected anyway given that previous version wasn't considered stable, ergo probably this should be released as a new major version (if this project follows semantic versioning).

To also fully disclose - I used LLM to generate:

  • Readme.md
  • History.markdown
  • .github/workflows.
  • appveyor.yaml

Would be grateful for review @DirtyF @ashmaroli @torrocus

Rest of code is written manually by me.

@galuszkak

Copy link
Copy Markdown
Author

Hi,

kind follow up @DirtyF @pathawks @ashmaroli @torrocus.

Thanks in advanced for review!

@galuszkak

Copy link
Copy Markdown
Author

Hi,

second kind follow up @DirtyF @pathawks @ashmaroli @torrocus

Thank you!

@mattr-

mattr- commented Jun 10, 2026

Copy link
Copy Markdown
Member

Appreciate the contribution! Please don't @-mention individual folks for review in the future. Most folks you mentioned no longer contribute to Jekyll on a regular enough basis for you to get a timely review, unfortunately.

After spending some time looking at this PR, I'm afraid that it tries to do too much and isn't explained well enough. The updates to the README are too broad. History.markdown is managed by automation and should not be modified. The CI change (both the Actions workflows and appveyor config) are not tightly scoped enough.

If you could tighten up the scope to only the things needed to support the new version of commonmarker, that would be the best path forward toward getting this merged.

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