[GPT-517] Exposing registry supported templates via context #423
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nickbalestra
changed the title
|
Want to add some tests for this |
matteofigus
requested changes
Mar 28, 2017
src/registry/domain/repository.js
Outdated
| }); | ||
| } | ||
| return []; | ||
| }, |
There was a problem hiding this comment.
This is good, but I am thinking performance here. This if+map+getInfo() is executed on each component execution (when the context is baked) by making this call and I don't think there is actually any need for that, as the templates are kind of statically initialised during registry's start anyways. What about moving this to the constructor on the top (around line 17) and memorising that in a variable, so that this method just exposes it?
There was a problem hiding this comment.
totally agree!
|
@matteofigus Added some test, and some error handling. Also removed the need to define core-templates in the conf (next will need to see how to extend this dinamically for dev). |
nickbalestra
changed the title
matteofigus
approved these changes
Mar 29, 2017
This was referenced Mar 31, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
oc-template-jadeandoc-template-handlebarsby defaultgetTemplate()helper for the repository to leverage on template.getInfo() API@matteofigus did I miss anything? (Still WIP)