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

Remove template simple? #1615

Open
ghost opened this issue May 4, 2021 · 6 comments
Open

Remove template simple? #1615

ghost opened this issue May 4, 2021 · 6 comments
Labels

Comments

@ghost
Copy link

ghost commented May 4, 2021

I would like to discuss if it would be possible to remove template::simple in favor of using the bundled implementation of template::tiny as the default template engine in Dancer2?

At the moment, the project is bundling both template::simple and template::tiny. The docs recommend users not to run template::simple in production, so it may make more sense to steer people towards template::tiny and template::toolkit.

If nothing else, the code would become slightly smaller by removing template::simple.

Any thoughts on this?

@ghost ghost changed the title Remove template simple Remove template simple? May 4, 2021
@cromedome
Copy link
Contributor

Honestly, I'd like to remove our forks of Template::Simple and Template::Tiny and just use Template::Tiny as it exists on CPAN. Our fork of the latter isn't even used.

There are some things that we need to work through first, and most of them affect people that may have built their own skeleton apps for use with dancer2 gen. I am hoping to make some progress on this soon.

Thanks for bringing this idea to our attention!

@ghost
Copy link
Author

ghost commented May 4, 2021

Sounds good to me.

I made a branch and removed Template::Simple, at least. Is it ok to send a pull request, for review?

@cromedome
Copy link
Contributor

Absolutely!

@ghost
Copy link
Author

ghost commented May 4, 2021

Great!
Just created one, #1616

@veryrusty
Copy link
Member

My 2 cents:

Dancer::Template::Simple isn't a fork of Template::Simple. It was written from scratch and unfortunately has the same name as another template module on CPAN.

Dancer2::Template::Simple is there to ease anyone using Dancer::Template::Simple to migrate to D2.

Dancer::Template::Tiny was never part of Dancer's core.

Dancer2::Template::Tiny includes a fork of Template::Tiny with extra functionality. The request to merge that into Template::Tiny has sat there for over 9 years, and with Ovid's fork for Template::Tiny::Strict in the past week, safe to assume will never be included.

If I had to remove one template engine from D2 core, I'd remove Dancer2::Template::Tiny. We can turn that into its own module on CPAN for those that need or want it.

If we remove Dancer2::Template::Simple, we need to add to migration docs what the alternatives are in case someone uses that in the D1 app and wants an upgrade path.

@ghost
Copy link
Author

ghost commented May 5, 2021

Good input, thank you!

Perhaps add to the migrations docs and set a deprecation time, if a decision is made to remove Template::Simple?

I can start adding to the migration docs in the pull request.

cromedome added a commit that referenced this issue May 8, 2021
This is one thing to come out of Issue #1615 on GitHub. Got me to
thinking about why we have two template systems in core when we really
want to steer devs to other template engines.

After talking with SysPete and veryrusty, it was decided we should
update the pod here to clarify what role Dancer2::Template::Simple
fills, and to consider splitting Dancer2::Template::Tiny to another
distribution.
cromedome added a commit that referenced this issue May 16, 2021
This is one thing to come out of Issue #1615 on GitHub. Got me to
thinking about why we have two template systems in core when we really
want to steer devs to other template engines.

After talking with SysPete and veryrusty, it was decided we should
update the pod here to clarify what role Dancer2::Template::Simple
fills, and to consider splitting Dancer2::Template::Tiny to another
distribution.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants