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

Make template system backward compatible #1173

Merged
merged 2 commits into from
Mar 31, 2020

Conversation

maartenbreddels
Copy link
Collaborator

Fixes #1158

This also makes nbconvert 6.0.0 compatible with voila (0.1.20 as of writing), and I guess many more projects.

The basic idea is to add templates with the old names in the compatibility subdirectory, which will be added to the search path. I've done this for rst.tpl and display_priority.tpl, but if people like this I can do it for the other old template filenames as well.

I think it does not bring an extra dev burden for this project while staying backward compatible.

Should we try to make it fully backward compatible, including running tests?

@MSeal
Copy link
Contributor

MSeal commented Jan 21, 2020

So I would like to not indefinitely support old patterns if we can avoid it. I think this is a good idea for the 6.0.0 release but it'd be nice if we could emit a deprecation warning stating the new path they should be using. Thoughts?

@MSeal
Copy link
Contributor

MSeal commented Feb 4, 2020

Pinging to refresh inboxes on PR conversation

@maartenbreddels
Copy link
Collaborator Author

So I would like to not indefinitely support old patterns if we can avoid it.

It would make many tools just work, it's 1 lines per template, I think it would make library owners happy.

We could add the templates, indeed, with a warning, but only remove it in version 7?

Or we do the most popular only? html/rst?

@MSeal
Copy link
Contributor

MSeal commented Feb 5, 2020

It would make many tools just work, it's 1 lines per template, I think it would make library owners happy.

I agree

We could add the templates, indeed, with a warning, but only remove it in version 7?

If it's easy to add the warning and have an extra line or two per template that does sound ideal. Maybe we should pick a release sooner than 7 to remove it (who know how many years that is)? 6.2? 6.3? Or maybe we just say no sooner than 6.2 to leave it open to extending removal?

@maartenbreddels
Copy link
Collaborator Author

I would say, we keep it to this, and we can add support for more old template names on a request/PR basis, using this as an example PR. This gives us some feedback on how people rely on it, and how they feel about migrating.

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.

Works for me -- I'm going to get #1211 merged and I think we can do a new alpha release. Any other changes you think we need to address before doing so?

@MSeal MSeal merged commit 0f41014 into jupyter:master Mar 31, 2020
@MSeal
Copy link
Contributor

MSeal commented Mar 31, 2020

Also docs build needs a fix -- some linting error in sphinx that's unrelated to this

@MSeal MSeal added this to the 6.0 milestone Sep 8, 2020
@maartenbreddels maartenbreddels deleted the feat_template_compatibility5 branch September 30, 2020 10:59
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.

Extending rst.tpl fails in current master and 6.0.0a0
2 participants