Skip to content

Added Template Factory#2

Merged
XedinUnknown merged 7 commits intodevelopfrom
task/template-factory
Jul 7, 2020
Merged

Added Template Factory#2
XedinUnknown merged 7 commits intodevelopfrom
task/template-factory

Conversation

@XedinUnknown
Copy link
Member

Adds a template factory, which simplifies the process of creating templates for its consumers. It creates all factories with the specified evaluator implementation (through injected factory), default context, and custom functions. This allows all templates to share a common set of data, which can be overridden at render time, and of functions.

@XedinUnknown XedinUnknown added the enhancement New feature or request label Feb 28, 2020
@XedinUnknown XedinUnknown added this to the 0.1 milestone Feb 28, 2020
@XedinUnknown XedinUnknown requested a review from mecha February 28, 2020 14:20
@XedinUnknown XedinUnknown self-assigned this Feb 28, 2020
- Changed build badge branch to `develop`.
- Removed Dhii standards badge.
Copy link
Member

@mecha mecha left a comment

Choose a reason for hiding this comment

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

LGTM. Just seems to be missing some docs in the code; minor details.

@XedinUnknown
Copy link
Member Author

Couldn't really find where the docs are missing. Could you kindly point out those places?

@XedinUnknown
Copy link
Member Author

@mecha, please take a look at the readme.

@XedinUnknown XedinUnknown requested a review from mecha April 8, 2020 17:22
Copy link
Member

@mecha mecha left a comment

Choose a reason for hiding this comment

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

Having forgotten the implementation, I read the README as a "new"-ish user wanting to learn how to use the library. Required multiple readings to get the gist of things. Here are some recommendations:

  1. Start off with simple things: creating a template and rendering a template. Opening with factories right off the bat is pretty disorienting. Present the reader with the bare minimum, and then build up from there.
  2. Demonstrate factories after point 1, showing off their benefits (like default ctx and functions).
  3. The bullet list under "Usage" means nothing to the reader until they read the remainder of the file. I found myself reading it and thinking "wtf are $c, $f and uc?"
  4. Avoid mentioning where code should be. Service definition, controller-level ... who cares?
  5. I'd remove unnecessary syntax like service-ish functions or immediately-invoked functions. They're not relevant to the library and just make the demonstrated code look more complex than it should be. It doesn't do the library justice.

@XedinUnknown
Copy link
Member Author

I'll improve the docs in a different PR.

@XedinUnknown XedinUnknown merged commit 7473f66 into develop Jul 7, 2020
@XedinUnknown XedinUnknown deleted the task/template-factory branch July 7, 2020 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants