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

Allow users to customize mathjax URLs #1609

Merged
merged 14 commits into from
Sep 2, 2021
Merged

Conversation

bsyouness
Copy link
Contributor

We're running into an issue where we'd like to customize the mathjax URL used by nbconvert but we're unable to do so easily. This PR uses the same methodology to make the mathjax customizable as is used to load the js scripts in the require.js jinja templates. This allows users to circumvent the use of CDNs to pull the mathjax dependencies.

@bsyouness
Copy link
Contributor Author

@bollwyvl @MSeal can I ask for a review? Not sure who to reach out to? Also the CI broke with this PR it looks like: #1599

@bsyouness
Copy link
Contributor Author

@SylvainCorlay could i get a review on this too please?

@jasongrout
Copy link
Member

@bsyouness - we just fixed the CI checks. Can you rebase or merge in main like you did above to get the CI checks working again?

@jasongrout jasongrout closed this Sep 1, 2021
@jasongrout jasongrout reopened this Sep 1, 2021
@bsyouness
Copy link
Contributor Author

bsyouness commented Sep 1, 2021

@jasongrout thanks for the review and for walking me through the comments! I made the changes we talked about.

@SylvainCorlay
Copy link
Member

@bsyouness I noticed that your git author email does not match your github account email, which causes github to not credit them to you. You may want to either change the author email for these commits or add that address to your github account to be properly credited before the PR gets in.

Copy link
Member

@jasongrout jasongrout left a comment

Choose a reason for hiding this comment

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

This looks great to me now. Thanks!

@jasongrout
Copy link
Member

jasongrout commented Sep 2, 2021

Since Youness and I both work at the same place, I'd prefer if someone else (@MSeal?) looks at this too and does the merge. I'm also happy to merge it if someone else approves.

Copy link
Contributor

@MSeal MSeal left a comment

Choose a reason for hiding this comment

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

LGTM as well

@MSeal MSeal merged commit 6c7c34a into jupyter:main Sep 2, 2021
@bsyouness bsyouness deleted the customizeajaxurl branch September 7, 2021 15:01
@jasongrout jasongrout added this to the 6.2 milestone Sep 13, 2021
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.

5 participants