Skip to content

Change initialization of JinjavaInterpreter to instantiation#347

Merged
mattcoley merged 1 commit into
HubSpot:masterfrom
Roobun:jinjava/instantiate
Jul 11, 2019
Merged

Change initialization of JinjavaInterpreter to instantiation#347
mattcoley merged 1 commit into
HubSpot:masterfrom
Roobun:jinjava/instantiate

Conversation

@Roobun

@Roobun Roobun commented Jun 26, 2019

Copy link
Copy Markdown
Contributor

Now it's possible to override instantiate method
It'll be helpful if you use extended interpreter

@mattcoley

Copy link
Copy Markdown
Collaborator

I believe you will also have to look at https://github.com/HubSpot/jinjava/blob/master/src/main/java/com/hubspot/jinjava/Jinjava.java which also creates interpreters.

@mattcoley

Copy link
Copy Markdown
Collaborator

Maybe we can use a factory pattern for the JinjavaInterpreter and set the factory on the JinjavaConfig. So there would be some interface InterpreterFactory with a default JinjavaInterpreterFactory that is set as a field on JinjavaConfig. Everywhere we create a new interpreter would instead use config.getInterpreterFactory().newInstance(...). You could then create your own factory to serve your own interpreters and set that value in the config.

@Roobun

Roobun commented Jul 11, 2019

Copy link
Copy Markdown
Contributor Author

Maybe we can use a factory pattern for the JinjavaInterpreter and set the factory on the JinjavaConfig. So there would be some interface InterpreterFactory with a default JinjavaInterpreterFactory that is set as a field on JinjavaConfig. Everywhere we create a new interpreter would instead use config.getInterpreterFactory().newInstance(...). You could then create your own factory to serve your own interpreters and set that value in the config.

Sure. Please take a look at new changes I added.

@Roobun Roobun force-pushed the jinjava/instantiate branch from 8cb3c7f to 1f68b2c Compare July 11, 2019 11:52

@mattcoley mattcoley left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM thanks!

@mattcoley

Copy link
Copy Markdown
Collaborator

I'll merge and do a release

@mattcoley mattcoley merged commit a96ebf9 into HubSpot:master Jul 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants