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 MathJax CDN link. #2714

Merged
merged 1 commit into from Apr 12, 2017
Merged

Update MathJax CDN link. #2714

merged 1 commit into from Apr 12, 2017

Conversation

@Anteru
Copy link
Contributor

Anteru commented Apr 11, 2017

Description

This fixes #2713 by updating the MathJax links to cdnjs and fixing it to version 2.7.0 (instead of latest).

Copy link
Contributor

felixfontein left a comment

LGTM

Copy link
Member

Kwpolska left a comment

Make it latest. Using a specific version puts a huge burden on us.

@Anteru

This comment has been minimized.

Copy link
Contributor Author

Anteru commented Apr 12, 2017

There's no latest on the new CDN.

@Anteru

This comment has been minimized.

Copy link
Contributor Author

Anteru commented Apr 12, 2017

Sphinx updated to 2.7.0 as well: sphinx-doc/sphinx@78148e9 -- there's no latest any more. Either you update to 2.7.0, or it just stops working for everyone in two weeks.

@felixfontein

This comment has been minimized.

Copy link
Contributor

felixfontein commented Apr 12, 2017

It won't (or at least, shouldn't) stop because of the redirect. But yes, it would be good to get this fixed ASAP.

@Kwpolska: how about injecting the MathJax version into the templates as a variable? (Maybe via global context?) Then at least there's only one place to update, not four. The big downside would be that everything rebuilds at the next update.

I also noticed that we have the same problem with KaTeX. There, version 0.6.0 is hardcoded in the templates (while the latest version is 0.7.1).

@Kwpolska

This comment has been minimized.

Copy link
Member

Kwpolska commented Apr 12, 2017

Putting it in GLOBAL_CONTEXT will result in rebuilding too much. I’ll bump KaTeX for now as well and merge.

Also, @Anteru: if you have a patch ready, just send a PR — otherwise, discussion might split between the two (I had to look for the announcement link in the other issue)

@Kwpolska Kwpolska merged commit 592e10e into getnikola:master Apr 12, 2017
1 of 3 checks passed
1 of 3 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
codacy/pr Hang in there, Codacy is reviewing your Pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Anteru

This comment has been minimized.

Copy link
Contributor Author

Anteru commented Apr 12, 2017

Sure, I just did this because the CONTRIBUTING.rst says "First, make sure there's an open issue", then create the branch and make a pull request. I wasn't sure if it's ok to just do a PR without the issue.

@Kwpolska

This comment has been minimized.

Copy link
Member

Kwpolska commented Apr 13, 2017

It’s okay, I’ve just reworded that section (commit a953fbb)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.