Am 2017-01-26 um 17:54 schrieb Claude Brisson:
On 26/01/2017 14:11, Michael Osipov wrote:
[...]
* StringUtils#normalizePath() can likely be replaced with Common IO's
FilenameUtils#normalize()
If we take this route, then we'll shade the commons-io class.
Absolutely, they might be other spots where this class can be used.
Moreover, either using IOUtils#closeQuietly() or try-with-resources will
clean up a lot of boilerplace code.
* OSGi metadata are gone from Core, no Bundle-SymbolicName, no
Exports, everything is gone from MANIFEST.MF
This I don't understand at all. You said "gone", so you implied that
those meta informations *were* to be found somewhere before this RC? I
think we never made the effort to properly populate those informations.
JIRA release notes say:
[VELOCITY-694] Make Velocity OSGi-ready
I do not see any of the OSGi required metadata.
* There are several spots where collections are iterated with a for
loop with a counter and #get(i) or a literal (legacy code). This
should generally be avoided. One example has been commented on GitHub.
I agree. Hopefully, this reenginering has already been done in sensitive
code sections, and remaining ones should be harmless.
Are you sure? I found a few #get(i) in a few loaders, RuntimeMacro and
Template. In a high-concurrency webapp where templates and macros are
loaded very often, this can be an issue, but of course, I do not have
hard numbers for this.
* Terrific improvement on the JavaCC code generation!
* JavaCC tells me:
Note: UNICODE_INPUT option is specified. Please make sure you create
the parser/lexer using a Reader with the correct character encoding.
Warning: Choice conflict in [...] construct at line 1325, column 21.
Expansion nested within construct and expansion following
construct
have common prefixes, one of which is: <TEXT>
Consider using a lookahead of 2 or more for nested expansion.
Warning: Choice conflict in (...)* construct at line 2157, column 5.
Expansion nested within construct and expansion following
construct
have common prefixes, one of which is: <WHITESPACE>
Consider using a lookahead of 2 or more for nested expansion.
Is this something one needs to take care of?
Not at all. Those two grammar constructs partially rely on the *order of
declaration* (and the '?' cardinality suffix inside a repetition
operator is enough to trigger the warning). javacc is just anxious about
that, and suggests adding lookahead instead.
OK, great. What about UNICODE_INPUT? Docs say we should set this if we
have Unicode files. We have set UTF-8. Can we ignore this message too#?
Thanks for all this remarks, Michael. They all are very constructive.
Very much welcome.
Now my question is: do you or someone else think that the OSGi missing
meta-informations are a show stopper for the 2.0? Otherwise, my plan is
to open JIRA issues for all those remarks, try to release the RC6, and
handle the issues in 2.0.1 or 2.1. The OSGi thing is the only one I'm
not really sure about, since I'm not using any tool relying on OSGi
myself (or maybe I'm the Monsieur Jourdain of the OSGi).
I do not use OSGi myself, but this says pretty much everything:
https://mvnrepository.com/artifact/org.apache.servicemix.bundles/org.apache.servicemix.bundles.velocity.
I consider proper OSGi metadata as vital for Velocity. Unfortunately, I
do not know what to set with the Bundle Maven Plugin to have proper data
generated.
Michael
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org
For additional commands, e-mail: dev-h...@velocity.apache.org
