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

Merge in outstanding pull requests for v2.5.0 release #242

Closed
jonbullock opened this Issue Nov 10, 2015 · 7 comments

Comments

Projects
None yet
2 participants
@jonbullock
Member

jonbullock commented Nov 10, 2015

I'm manually merging in the outstanding pull requests marked with v2.5.0 in a separate branch (merge-2-5-0-prs) at the moment and I've created this issue to track my progress and list the outstanding clean up tasks.

@jonbullock jonbullock added the task label Nov 10, 2015

@jonbullock jonbullock self-assigned this Nov 10, 2015

@jonbullock jonbullock added this to the v2.5.0 milestone Nov 10, 2015

@jonbullock

This comment has been minimized.

Member

jonbullock commented Nov 10, 2015

Pull requests to merge in:

  • #186 Pagination
  • #175 Simplify and aggregate rendering testing
  • #185 Introduced Groovy MarkupTemplateEngine
  • #161 Added support for Pebble template engine
  • #166 Jade template engine implementation
  • #154 Support for Jade template engine
  • #135 Preliminary refactoring
  • #138 Extracted various rendering modes to allow their customisation
  • #120 WIP: Rework commandline
  • #104 Extensonless URLs

Clean up tasks so far:

  • Merge FreemarkerRendererTest into abstract system and remove FreemarkerRendererTest
  • Duplication of freemarker templates in tests (/templates & /freemarkerTemplates) needs to be solved
  • Rename GroovyTemplateEngine class to GroovySimpleTemplateEngine class and associated tests (or should other name considering the engine uses Simple & Xml)
  • Use static references to data model variable names in GroovyMarkupTemplateEngine & PebbleTemplateEngine
  • Either merge RendererSpec.groovy into exiting Java unit tests or keep it and make sure there is no overlap between existing Java unit tests and this
  • Move PebbleRendererTest class into /template package and update so matches other engine tests
  • Deal with assembly-example-project-pebble.xml not linked up to relevant source (not used in pom.xml)
  • Use static references to data model variable names in JadeTemplateEngine
  • Deal with assembly-example-project-jade.xml not pointing to any files?
  • Merge JadeRendererTest into abstract system
  • Move JadeRendererTest class into /template package and update to matches other engine tests
  • Arrange to move github.com/mariuszs/jbake-example-project-jade to JBake org
  • Apply ModelExtractors to new template engines (Groovy Markup, Pebble and Jade)
  • Review uri.noExtension option and decide if it should be converted into multiple options (enable flag and path matcher)
@haumacher

This comment has been minimized.

Contributor

haumacher commented Nov 11, 2015

Don't you think that #219 "Detailed errors with stack trace of the original problem" isn't also a normative candidate for such a cleanup release?

A part of my wikibake experiments (#227) I've also introduced some additional abstractions around the document model handling. These refactorings have no influence to the application functionality, but the wikibaking heavily depends on it. If I had the chance of getting these refactorings integrated into a pending release, I could extract them to a separate PR to make the outstanding wikibake changes smaller.

@jonbullock

This comment has been minimized.

Member

jonbullock commented Nov 11, 2015

Thanks for reminding me, I've scheduled #219 in for v2.5.0.

Please be aware there are some significant changes to the code in the pull requests above.

jonbullock added a commit that referenced this issue Nov 18, 2015

@jonbullock

This comment has been minimized.

Member

jonbullock commented Jun 9, 2016

@haumacher could you direct me to the abstractions you're referring to?

@haumacher

This comment has been minimized.

Contributor

haumacher commented Jun 12, 2016

@jonbullock I primarily mean the encapsulation of the document properties that were represented as Map<String,Object> into a separate class JDocument. And additionally using Java constants for all these document properties that are scattered all over the code as String literals (see haumacher@87d0072).

But there are more refactorings done in https://github.com/haumacher/jbake/commits/wikibake.1?page=1

@jonbullock

This comment has been minimized.

Member

jonbullock commented Jun 14, 2016

@haumacher I can see that being a benefit for future plans so if you're willing to extract that into a separate PR I'd be happy to include it. Most of those string literals have been dealt with as part of 2474ad9 but they always felt out of place in Crawler so putting them into JDocument seems like the right place.

@jonbullock jonbullock closed this Jun 14, 2016

@jonbullock

This comment has been minimized.

Member

jonbullock commented Jun 14, 2016

The PR's listed in this issue have now been merged and clean up tasks performed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment