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

Update GFM release to 0.29.0.gfm.2 #148

Merged
merged 3 commits into from Sep 17, 2021

Conversation

phillmv
Copy link
Collaborator

@phillmv phillmv commented Sep 16, 2021

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 introduced0.29.0.gfm.1, which fixes a security vulnerability.

I generated this PR via the following:

$ git clone --recurse-submodules git@github.com:gjtorikian/commonmarker.git
$ git checkout -b update-to-0290gfm2
  # first, comment out the `git pull` line in ./script/update_submodules,
  # cos I'm referencing a tag & it gave an error
$ ./script/update_submodules 0.29.0.gfm.2
$ git remote add phillmv git@github.com:phillmv/commonmarker.git
$ git push phillmv update-to-0290gfm2

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 and
  • ext/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 the HtmlRenderer 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 from commonmarker's POV.

Copy link
Collaborator

@kivikakk kivikakk left a 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!

@gjtorikian
Copy link
Owner

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 gjtorikian merged commit f30f525 into gjtorikian:main Sep 17, 2021
@phillmv
Copy link
Collaborator Author

phillmv commented Sep 17, 2021

@gjtorikian thank you! much obliged.

@phillmv Since presumably GitHub still uses this, do you want write access to this repo and the gem?

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.

@digitalmoksha
Copy link
Contributor

Cool, thanks @phillmv and @gjtorikian! Any possibility of pushing a new version of the gem?

@phillmv
Copy link
Collaborator Author

phillmv commented Sep 17, 2021

@digitalmoksha I thought @gjtorikian just pushed it? see 0.23.2 -> https://rubygems.org/gems/commonmarker/versions/0.23.2

@digitalmoksha
Copy link
Contributor

@phillmv yep your'e right 🤦 I was looking at the Releases page

Perfect, thanks!

@gjtorikian
Copy link
Owner

Ps. I've put in a note re: sponsorship but alas i haven't control of the purse strings.

Well, thank you for trying. <3

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.

None yet

4 participants