Am 2019-06-17 um 16:23 schrieb Claude Brisson:
Thanks a lot for your review, Michael.
I think I have handled all the points you raised, except otherwise noted.
> I don't like the fast that in several classes the to-be-replaced
symbols still appear, e.g., RuntimeInstance
The RuntimeService now only exposes a getParserConfiguration() method.
> Since you successfully abstracted from the tokens, I don't see reason
to still call them by name, .e.g., char.dollar. I would call them by
function -- even it makes it a bit longer, it is more descriptive. I
hope you get the point.
Outside the parser, getting a specific char is done with :
runtimeServices.getParserConfiguration().getDollarChar()
Inside the parser, I'm fine with parser.dollar field access. It's not
exposed to the user, and at least I'm certain to minimize the overhead.
> + <artifactId>maven-bundle-plugin</artifactId>
> Why do we need this on an example?
Because otherwise, I get:
[ERROR] Failed to execute goal
org.apache.maven.plugins:maven-jar-plugin:3.1.1:jar (default-jar) on
project velocity-custom-parser-example: Error assembling JAR: Manifest
file:
/home/claude/Projects/velocity/engine/parser/velocity-custom-parser-example/target/classes/META-INF/MANIFEST.MF
does not exist.
It does so even if I try to have maven-jar-plugin skip parent
configuration.
Thank you Claude, I will review again by the end of the week.
Michael
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org
For additional commands, e-mail: dev-h...@velocity.apache.org
