perwendel / spark-template-engines

Repository for different Template engine implementations.
Apache License 2.0
134 stars 101 forks source link

Huge update to all template engines. #40

Closed N0odlez closed 8 years ago

N0odlez commented 8 years ago

Added license to all engines. Unified all POMs Created tests for all engines to test the rendering is working. Updated all dependencies bar JetBrick that is on version 2.1.1 due to a bug. Updated and created examples for all engines. Updated and created README's for all engines. Started to update JavaDocs.

tipsy commented 8 years ago

Thanks, this is great. Will start reviewing tonight.

tipsy commented 8 years ago

Overall, great work! Some notes:

N0odlez commented 8 years ago

Great, I'll get those sorted and if you don't mind with the example. I'm not exactly good at explaining things in words.

tipsy commented 8 years ago

The tests also seem a bit excessive. Why do need to start a server and test the routing too? Maybe I'm missing something, but to me it looks like:

String rendered = new FreeMarkerTemplateEngine().render(new ModelAndView(map, "hello.ftl"));
Assert.assertEquals(rendered , expected);

Should be enough?

tipsy commented 8 years ago

Some of the tests are called TestRender , while others are called RenderTest.

N0odlez commented 8 years ago

I'll get all of that sorted. I don't think of testing it that way... always overcomplicating things ahah.

N0odlez commented 8 years ago

Hmmm.. Made all the changes and the building passes the tests in both intellij and maven but just not with Travis?

tipsy commented 8 years ago

Are you using windows? \r\n might be causing issues. Try simplifying the test template to something like The ${templateEngineName} template-engine works!, you don't really need HTML and newlines to test.

N0odlez commented 8 years ago

ok ill change that and force push again. Do you want me to squash the commits as well?

tipsy commented 8 years ago

Do you want me to squash the commits as well?

That would be nice, yes!

N0odlez commented 8 years ago

Hmm problem, because jade takes the first string of letters as a html node I'm just going to add <h1> to all just so this will work.

tipsy commented 8 years ago

@AzureusNation, did you manage to sort it out?

N0odlez commented 8 years ago

Sorry yes it's all done. Just been busy at work.

tipsy commented 8 years ago

Fixing this now