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

Support Commonmarker v1 api #396

Closed
wants to merge 1 commit into from
Closed

Support Commonmarker v1 api #396

wants to merge 1 commit into from

Conversation

unasuke
Copy link

@unasuke unasuke commented Dec 29, 2023

What

The Commonmarker gem released v1 incluging breaking api changes.
https://github.com/gjtorikian/commonmarker/releases/tag/v1.0.0

These changes supports the new commonmarker's api.

To support commonmaker's breaking api changes, tilt gem will release new version including incompatible api change also.
For notice to tilt gem user, is it better to check commonmarker gem's version?

The Commonmarker gem released v1 incluging breaking api changes.
https://github.com/gjtorikian/commonmarker/releases/tag/v1.0.0

These changes supports the new commonmarker's api.
@jeremyevans
Copy link
Contributor

Thank you very much for the patch. However, the changes to tilt must be backwards compatible and continue to work with the older commonmarker API. This is especially true since commonmarker >= 1:

  • Switched the backend from C to Rust, and Rust does not support as many platforms as C.
  • Only supports Ruby >= 3.1

In tilt/commonmarker, you'll need to check the commonmarker version after requiring it, and make the appropriate changes so it works with both versions. Do you think you'd be able to make those changes?

@unasuke
Copy link
Author

unasuke commented Dec 29, 2023

In tilt/commonmarker, you'll need to check the commonmarker version after requiring it, and make the appropriate changes so it works with both versions. Do you think you'd be able to make those changes?

I'll write a patch that works for both versions of commonmaker. But it's hard to write a test code (prepare test environment) I think.

@jeremyevans
Copy link
Contributor

In tilt/commonmarker, you'll need to check the commonmarker version after requiring it, and make the appropriate changes so it works with both versions. Do you think you'd be able to make those changes?

I'll write a patch that works for both versions of commonmaker. But it's hard to write a test code (prepare test environment) I think.

I just noticed, but you filed this against the rtomayko repository, which is no longer used. You need to submit the pull request to https://github.com/jeremyevans/tilt. That has a CI that runs for pull requests. CI should be able to check if it works with both versions of commonmarker. Just remove the version restriction for commonmarker in .ci.gemfile in the pull request.

@unasuke
Copy link
Author

unasuke commented Dec 29, 2023

You need to submit the pull request to https://github.com/jeremyevans/tilt.

Oops. I forgot to check the repository url on rubygems.org. Thanks!

@unasuke unasuke closed this Dec 29, 2023
@unasuke unasuke deleted the commonmarker-v1 branch December 29, 2023 16:30
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