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 mustache4dart copy inside dartdoc and use 'mustache' pub package #1894

Merged
merged 10 commits into from
Jan 9, 2019

Conversation

jcollins-g
Copy link
Contributor

mustache4dart hasn't been fully supported on Dart 2 for a while, so I looked into porting to a different mustache implementation. It turned out not to be that hard, and this mustache implementation is slightly faster, published with full Dart 2 compatibility, while being less forgiving of Dartdoc's malformed templates.

I've fixed the bugs in our templates here and with one small feature to the mustache library
(xxgreg/mustache#41) the port is good to go.

@googlebot googlebot added the cla: yes Google CLA check succeeded. label Jan 9, 2019
@jcollins-g
Copy link
Contributor Author

We will have to do the DEPS/mirrors dance with the SDK build but I think it is worth it to clean up dartdoc's dependencies.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.8%) to 93.488% when pulling 5a77e83 on mustache-shave into 11abbbc on master.

@jcollins-g jcollins-g requested review from devoncarew and pq January 9, 2019 20:31
Copy link
Member

@devoncarew devoncarew left a comment

Choose a reason for hiding this comment

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

Great to remove the local copy of mustache2dart. And it looks like the format of the files is nearly the same for the two template impls.

@jcollins-g jcollins-g merged commit ea0392e into master Jan 9, 2019
@jcollins-g jcollins-g deleted the mustache-shave branch January 9, 2019 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google CLA check succeeded.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants