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

Upgrade to jekyll 3.9 #2570

Merged
merged 8 commits into from
Aug 14, 2020
Merged

Upgrade to jekyll 3.9 #2570

merged 8 commits into from
Aug 14, 2020

Conversation

johnpryan
Copy link
Contributor

@johnpryan johnpryan commented Aug 11, 2020

Upgrades to kramdown 2.3.0, which patches CVE-2020-14001

This change runs bundle update. When I ran bundle list I see that
kramdown 2.3.0 is now installed, (although the Gemfile.lock says
kramdown (>= 1.17, < 3))

@googlebot googlebot added the cla: yes Contributor has signed the Contributor License Agreement label Aug 11, 2020
@johnpryan
Copy link
Contributor Author

I applied the same fix to the Flutter site here: flutter/website#4466

@johnpryan
Copy link
Contributor Author

@kwalrath
Copy link
Contributor

It looks like code excerpts don't work. :( E.g., look at the bottom of the homepage and the top of https://jr-dartlang-site.firebaseapp.com/guides/language/language-tour.

I think Patrice used to generate diffs (with the footer fixed to avoid unnecessary diffs due to date changes) to catch this sort of thing.

@johnpryan
Copy link
Contributor Author

Still looking at this... It's not clear to me yet if _plugins/markdown_with_code_excerpts.rb is throwing an exception or not running at all.

Upgrades to kramdown 2.3.0, which patches CVE-2020-14001
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-14001

This change runs `bundle update`. When I ran `bundle list` I see that
kramdown 2.3.0 is now installed, although the Gemfile.lock says
kramdown (>= 1.17, < 3)
This now matches the setup in flutter/website

Without this plugin, <?code-excerpt?> markers don't get processed in
Jekyll 3.9.
@johnpryan
Copy link
Contributor Author

@kwalrath I think this is working now. I haven't fully checked all of the snippets, but I've parsed through the Language Tour and Effective Dart and don't see any issues.

staged:

@kwalrath
Copy link
Contributor

Does the embedded DartPad on the homepage work for you? I'm still seeing code-excerpt instead of code, but I seem to recall that caching can be a problem (even though I cleared the browser cache).

image

@kwalrath
Copy link
Contributor

kwalrath commented Aug 13, 2020

Everything else is looking great, so far, including red & green backgrounds, highlighting, and codelabs with embedded DartPads.

@johnpryan
Copy link
Contributor Author

good catch, yeah that needs to be fixed.

@johnpryan
Copy link
Contributor Author

#2124 is broken after upgrading to 3.9. We could either revert that change (stop testing these snippets), leave the site on Jekyll 3.8.6, or figure out some other way to test the snippets.

@kwalrath
Copy link
Contributor

Oy, testing the snippets recently found a problem: https://github.com/dart-lang/site-www/pull/2575/files#diff-5516df53c13434e15d4ca7bd4fb9cc8b. Although I wonder if you knew about it but wanted to keep the code simple, since it doesn't cause a visible error.

How about we NOT auto-include the snippets for now, but file a bug to reenable auto-including them (or at least syncing the code so the latest version is always tested).

@johnpryan
Copy link
Contributor Author

Sounds good to me

Upgrading to Jekyll 3.9 caused the <?code-excerpt> tags to be
transformed into paragraph elements. Running the same Jekyll Converter
to process code excerpts fixed this issue. However, the snippets on
the homepage. By default the snippets in _try-dart-examples.md don't
get processed by this converter. Removing the underscore gets them
to be processed by this converter, but are added to the homepage as
prettified snippets, which isn't something dartpad_picker can use.

This is a temporary fix to allow us to upgrade to Jekyll 3.9:
#2578
@johnpryan
Copy link
Contributor Author

johnpryan commented Aug 14, 2020

Deployed: https://jr-dartlang-site.firebaseapp.com I needed to empty cache and reload to see the change, though.

Copy link
Contributor

@kwalrath kwalrath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for all the work, @johnpryan!

@kwalrath kwalrath merged commit e5609ec into dart-lang:master Aug 14, 2020
kwalrath pushed a commit that referenced this pull request Aug 31, 2020
This should have been removed as part of #2570

fixes #2604
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Contributor has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants