-
-
Notifications
You must be signed in to change notification settings - Fork 85
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
Update GFM release to 0.29.0.gfm.2
#148
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM — thank you!
Wow, sweet--thank you very much. @phillmv Since presumably GitHub still uses this, do you want write access to this repo and the gem? |
@gjtorikian thank you! much obliged.
Sure, I'll take it. Here's my newly created rubygems account. This is where I confess I that I did most of this work in my free time, and that whoever will worry about this in the future will probably not be me. In the long term, we'll have to do something so no one has to bother y'all to get releases out, but in the short term giving me write access will paper over any gaps. Thanks again, @gjtorikian & @kivikakk! Ps. I've put in a note re: sponsorship but alas i haven't control of the purse strings. |
Cool, thanks @phillmv and @gjtorikian! Any possibility of pushing a new version of the gem? |
@digitalmoksha I thought @gjtorikian just pushed it? see 0.23.2 -> https://rubygems.org/gems/commonmarker/versions/0.23.2 |
@phillmv yep your'e right 🤦 I was looking at the Releases page Perfect, thanks! |
Well, thank you for trying. <3 |
Hey @gjtorikian!
We just pushed a new release to cmark-gfm, and it'd be cool to be able to use it in commonmarker. Looking over at the changes introduced, looks like we'll also be including changes introduced
0.29.0.gfm.1
, which fixes a security vulnerability.I generated this PR via the following:
and then opened this PR.
Skimming the diff, this seems to have worked just fine and dandy; all the footnote changes I've worked in have made it in.
In addition, it looks like:
ext/commonmarker/ext_scanners.c
andext/commonmarker/table.c
are associated with this pull request github/cmark-gfm@85d8952
Thanks for taking a look!
edit: I realized that of course there was a footnotes test. I fixed the output check for
.to_html
/render_html
but figured that updating theHtmlRenderer
could be left outside of the scope of this PR.I also took the liberty of bumping the version to
0.23.2
, on the assumption that there is no major API change fromcommonmarker
's POV.