You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@velocity.apache.org by Claude Brisson <cl...@renegat.net> on 2017/01/16 14:02:43 UTC
[VOTE] Engine 2.0 RC5 Release quality
The Velocity Engine 2.0 RC5 is available.
Main changes since the RC4:
* the default encoding is now UTF-8 (and not the platform default)
* commons-collections is not any more a compilation dependency
* commons-lang3 dependency is not any more shaded
* the configuration API doesn't use ExtProperties anymore
* the events API has been reviewed: all events do receive the current
Context as argument
Release notes:
*
https://dist.apache.org/repos/dist/dev/velocity/velocity-engine/2.0/release-notes.html
Distribution:
* https://dist.apache.org/repos/dist/dev/velocity/velocity-engine/2.0/
Maven 2 staging repository:
*
https://repository.apache.org/content/repositories/orgapachevelocity-1015
If you have had a chance to review the test build, please respond with a
vote on its quality:
[ ] Leave at test build
[ ] Alpha
[ ] Beta
[ ] General Availability (GA)
Everyone who has tested the build is invited to vote. Votes by PMC
members are considered binding. A vote passes if there are at least
three binding +1s and more +1s than -1s.
Claude
Re: [VOTE] Engine 2.0 RC5 Release quality
Posted by Claude Brisson <cl...@renegat.net>.
Changing my vote to
[X] Leave at test build
after Michael remarks (sigh...).
On 16/01/2017 15:04, Claude Brisson wrote:
> And my vote:
>>
>> [ ] Leave at test build
>> [ ] Alpha
>> [ ] Beta
>> [X] General Availability (GA)
>
>
> Claude
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> For additional commands, e-mail: dev-help@velocity.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: [VOTE] Engine 2.0 RC5 Release quality
Posted by Claude Brisson <cl...@renegat.net>.
And my vote:
>
> [ ] Leave at test build
> [ ] Alpha
> [ ] Beta
> [X] General Availability (GA)
Claude
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: [VOTE] Engine 2.0 RC5 Release quality
Posted by Claude Brisson <cl...@renegat.net>.
>> I totally agree. I left things as I found them basically because I had
>> enough other things to learn about Maven by the time. If you want to
>> help further on this one you are welcome.
>
> I will have a look, especially at the patching of the generated source
> code. I assume that this is absolutely necessary.
>
Maybe not. I'm playing with it. Looks easier than I thought.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: [VOTE] Engine 2.0 RC5 Release quality
Posted by Michael Osipov <mi...@apache.org>.
Am 2017-01-16 um 22:56 schrieb Claude Brisson:
> That was fast.
>
> On 16/01/2017 22:29, Michael Osipov wrote:
>>
>> Here are my comments referred in my the previous mail:
>>
>>
>> * POM: use of JavaCC Maven Plugin: never ever add auto-generated code
>> to src/, it completely breaks
>> the agreed convention. Have it in target/ and attach the source code
>> with Buildhelper Maven Plugin, if
>> the JavaCC Plugin doesn't do it on its own.
> I totally agree. I left things as I found them basically because I had
> enough other things to learn about Maven by the time. If you want to
> help further on this one you are welcome.
I will have a look, especially at the patching of the generated source
code. I assume that this is absolutely necessary.
>> * POM: The deletion/creation of the JavaCC parser should rather be a
>> complete profile rather than
>> a hack with properties passed to Ant, if this is necessary at all. I
>> see no difference here to code generation by JAXB's XJC or Modello, etc.
>
> We could also regererate it each time. It's not that painful.
Absolutely, this is cheap. At best, the plugin would have a stale
detection based on mtime, but this is out of our business.
>> * There are a few null argument checks which can be better handled with Common Lang's Validate class
> Where, and why?
See:
> $ grep -r "throw new IllegalAr" .
> ./velocity-engine-core/src/main/java/org/apache/velocity/io/VelocityWriter.java: throw new IllegalArgumentException("Buffer size <= 0");
> ./velocity-engine-core/src/main/java/org/apache/velocity/runtime/directive/RuntimeMacro.java: throw new IllegalArgumentException("Null arguments");
> ./velocity-engine-core/src/main/java/org/apache/velocity/util/ArrayIterator.java: throw new IllegalArgumentException(
> ./velocity-engine-core/src/main/java/org/apache/velocity/util/ExtProperties.java: throw new IllegalArgumentException('\'' + token + "' does not contain " + "an equals sign");
> ./velocity-engine-core/src/main/java/org/apache/velocity/util/introspection/IntrospectorBase.java: throw new IllegalArgumentException ("class object is null!");
> ./velocity-engine-core/src/main/java/org/apache/velocity/util/introspection/IntrospectorBase.java: throw new IllegalArgumentException("params object is null!");
> ./velocity-engine-core/src/main/java/org/apache/velocity/util/introspection/IntrospectorBase.java: throw new IllegalArgumentException("class object is null!");
> ./velocity-engine-core/src/main/java/org/apache/velocity/util/introspection/IntrospectorCache.java: throw new IllegalArgumentException("class is null!");
> ./velocity-engine-core/src/main/java/org/apache/velocity/util/introspection/IntrospectorCache.java: throw new IllegalArgumentException("class is null!");
It basically does all you need, null with NPE, invalid values with IAE,
states with ISE according to today's Java exception conventions. Plus,
less brevity and boilerplate code.
Sample commit:
https://github.com/apache/maven/commit/618e62dd3315b4cb5b0f7dcdd37fc787c44b2ade
>> * Stuff like BUILD_README.txt absolutely do not belong to
>> src/main/java, see second point
>> * There are some unused imports once in a while
>> * Rather use maven.compiler.source and maven.compiler.target
>> properties to avoid duplicate value setting, checkstyle, findbugs,
>> JavaCC, etc.
> You mean between different pom files? I don't see several occurrences of
> findbugs, javacc, etc... and none of checkstyle (should there be any?).
Yes, between different ones. See m-compiler-p in parent and
javacc-maven-plugin core. If you add PMD too, you'll get three spots for
updating.
>> * If a dependency is used several times, e.g. SFL4J, it shall be added
>> to the dep mngt section of the parent POM to avoid repetition of
>> version elements. Hint: this can be done with reactor modules too
>>
> It's version has been factorized in the parent.
This isn't what was referring to. You still have repetitive clutter like
<version>${....version>}</version> whereas the better way is to like
here:
https://git-wip-us.apache.org/repos/asf?p=maven-wagon.git;a=blob;f=pom.xml;h=006fbe9bcf505e46e26a9e3906966262547e1e2b;hb=HEAD#l284
Modules only provide groupId and artifactId, the rest in inherited.
>> There are likely to be more issues I have missed.
>>
>> What is the benefit of velocity-engine-assembly actually? I know, ages
>> ago Velocity and other software was delivered as a tarball or zip file
>> with all JARs and the rest, including source. I think this is pretty
>> much obsolete now and partially redundant:
>> * Apache Parent already provides predefined descriptors which create a
>> source-release.zip for you, no further work necessary
>> ** source-release.zip is used for release testing/votes, as well as
>> for clients/users who do not want to clone from Git or checkout via
>> Subversion
>> * Binary artifacts are always available on Maven Central, I hardly
>> believe that someone will really download dist.zip to get the JARs and
>> add them manually somewhere, even if, they can get them from
>> search.maven.org
>>
> I'm +1 on this one. Let's drop assembly.
+1
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: [VOTE] Engine 2.0 RC5 Release quality
Posted by Claude Brisson <cl...@renegat.net>.
That was fast.
On 16/01/2017 22:29, Michael Osipov wrote:
>
> Here are my comments referred in my the previous mail:
>
>
> * README.txt requires a rewrite, it still talks about Ant, old
> directory structures, and much more
Ok.
>
> * POM: use of JavaCC Maven Plugin: never ever add auto-generated code
> to src/, it completely breaks
> the agreed convention. Have it in target/ and attach the source code
> with Buildhelper Maven Plugin, if
> the JavaCC Plugin doesn't do it on its own.
I totally agree. I left things as I found them basically because I had
enough other things to learn about Maven by the time. If you want to
help further on this one you are welcome.
> * POM: The deletion/creation of the JavaCC parser should rather be a
> complete profile rather than
> a hack with properties passed to Ant, if this is necessary at all. I
> see no difference here to code generation by JAXB's XJC or Modello, etc.
We could also regererate it each time. It's not that painful.
> * There are several spots which
> ** don't use SLF4J's {} placeholder,
> ** don't use System.lineSeparator() but '\n'
> ** do a toLowerCase() (e.g., #sameEncoding()) instead of
> toLowerCase(Locale.ROOT)
> * There is a lot trailing whitespace throughout the code
> * Generally, don't use log#isXXXEnabled() because of SLF4J's {}
> placeholders' foo
I'll check those ones.
> * There are a few null argument checks which can be better handled
> with Common Lang's Validate class
Where, and why?
> * StringBuffer is used sometimes where StringBuilder would suffice
> (w/o synchronization)
Ok.
> * Stuff like BUILD_README.txt absolutely do not belong to
> src/main/java, see second point
> * There are some unused imports once in a while
> * Rather use maven.compiler.source and maven.compiler.target
> properties to avoid duplicate value setting, checkstyle, findbugs,
> JavaCC, etc.
You mean between different pom files? I don't see several occurrences of
findbugs, javacc, etc... and none of checkstyle (should there be any?).
> * If a dependency is used several times, e.g. SFL4J, it shall be added
> to the dep mngt section of the parent POM to avoid repetition of
> version elements. Hint: this can be done with reactor modules too
>
It's version has been factorized in the parent.
> There are likely to be more issues I have missed.
>
> What is the benefit of velocity-engine-assembly actually? I know, ages
> ago Velocity and other software was delivered as a tarball or zip file
> with all JARs and the rest, including source. I think this is pretty
> much obsolete now and partially redundant:
> * Apache Parent already provides predefined descriptors which create a
> source-release.zip for you, no further work necessary
> ** source-release.zip is used for release testing/votes, as well as
> for clients/users who do not want to clone from Git or checkout via
> Subversion
> * Binary artifacts are always available on Maven Central, I hardly
> believe that someone will really download dist.zip to get the JARs and
> add them manually somewhere, even if, they can get them from
> search.maven.org
>
I'm +1 on this one. Let's drop assembly.
>
> Michael
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
> For additional commands, e-mail: dev-help@velocity.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: [VOTE] Engine 2.0 RC5 Release quality
Posted by Michael Osipov <mi...@apache.org>.
Am 2017-01-16 um 15:02 schrieb Claude Brisson:
> The Velocity Engine 2.0 RC5 is available.
>
> Main changes since the RC4:
>
> * the default encoding is now UTF-8 (and not the platform default)
> * commons-collections is not any more a compilation dependency
> * commons-lang3 dependency is not any more shaded
> * the configuration API doesn't use ExtProperties anymore
> * the events API has been reviewed: all events do receive the current
> Context as argument
>
> Release notes:
>
> *
> https://dist.apache.org/repos/dist/dev/velocity/velocity-engine/2.0/release-notes.html
>
>
> Distribution:
>
> * https://dist.apache.org/repos/dist/dev/velocity/velocity-engine/2.0/
>
> Maven 2 staging repository:
>
> *
> https://repository.apache.org/content/repositories/orgapachevelocity-1015
>
> If you have had a chance to review the test build, please respond with a
> vote on its quality:
>
> [ ] Leave at test build
> [ ] Alpha
> [ ] Beta
> [ ] General Availability (GA)
>
>
> Everyone who has tested the build is invited to vote. Votes by PMC
> members are considered binding. A vote passes if there are at least
> three binding +1s and more +1s than -1s.
Here are my comments referred in my the previous mail:
* README.txt requires a rewrite, it still talks about Ant, old directory
structures, and much more
* POM: use of JavaCC Maven Plugin: never ever add auto-generated code to
src/, it completely breaks
the agreed convention. Have it in target/ and attach the source code
with Buildhelper Maven Plugin, if
the JavaCC Plugin doesn't do it on its own.
* POM: The deletion/creation of the JavaCC parser should rather be a
complete profile rather than
a hack with properties passed to Ant, if this is necessary at all. I
see no difference here to code generation by JAXB's XJC or Modello, etc.
* There are several spots which
** don't use SLF4J's {} placeholder,
** don't use System.lineSeparator() but '\n'
** do a toLowerCase() (e.g., #sameEncoding()) instead of
toLowerCase(Locale.ROOT)
* There is a lot trailing whitespace throughout the code
* Generally, don't use log#isXXXEnabled() because of SLF4J's {}
placeholders' foo
* There are a few null argument checks which can be better handled with
Common Lang's Validate class
* StringBuffer is used sometimes where StringBuilder would suffice (w/o
synchronization)
* Stuff like BUILD_README.txt absolutely do not belong to src/main/java,
see second point
* There are some unused imports once in a while
* Rather use maven.compiler.source and maven.compiler.target properties
to avoid duplicate value setting, checkstyle, findbugs, JavaCC, etc.
* If a dependency is used several times, e.g. SFL4J, it shall be added
to the dep mngt section of the parent POM to avoid repetition of version
elements. Hint: this can be done with reactor modules too
There are likely to be more issues I have missed.
What is the benefit of velocity-engine-assembly actually? I know, ages
ago Velocity and other software was delivered as a tarball or zip file
with all JARs and the rest, including source. I think this is pretty
much obsolete now and partially redundant:
* Apache Parent already provides predefined descriptors which create a
source-release.zip for you, no further work necessary
** source-release.zip is used for release testing/votes, as well as for
clients/users who do not want to clone from Git or checkout via Subversion
* Binary artifacts are always available on Maven Central, I hardly
believe that someone will really download dist.zip to get the JARs and
add them manually somewhere, even if, they can get them from
search.maven.org
Michael
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: [VOTE] Engine 2.0 RC5 Release quality
Posted by Michael Osipov <mi...@apache.org>.
Am 2017-01-16 um 15:02 schrieb Claude Brisson:
> The Velocity Engine 2.0 RC5 is available.
>
> Main changes since the RC4:
>
> * the default encoding is now UTF-8 (and not the platform default)
> * commons-collections is not any more a compilation dependency
> * commons-lang3 dependency is not any more shaded
> * the configuration API doesn't use ExtProperties anymore
> * the events API has been reviewed: all events do receive the current
> Context as argument
>
> Release notes:
>
> *
> https://dist.apache.org/repos/dist/dev/velocity/velocity-engine/2.0/release-notes.html
>
>
> Distribution:
>
> * https://dist.apache.org/repos/dist/dev/velocity/velocity-engine/2.0/
>
> Maven 2 staging repository:
>
> *
> https://repository.apache.org/content/repositories/orgapachevelocity-1015
>
> If you have had a chance to review the test build, please respond with a
> vote on its quality:
>
> [ ] Leave at test build
> [ ] Alpha
> [ ] Beta
> [ ] General Availability (GA)
>
>
> Everyone who has tested the build is invited to vote. Votes by PMC
> members are considered binding. A vote passes if there are at least
> three binding +1s and more +1s than -1s.
I have started a review based on a diff between 1.7 and trunk. I will
report shortly. There are already some issues with the POM which should
be improved.
Michael
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@velocity.apache.org
For additional commands, e-mail: dev-help@velocity.apache.org
Re: [VOTE] Engine 2.0 RC5 Release quality
Posted by Michael Osipov <mi...@apache.org>.
Am 2017-01-16 um 15:02 schrieb Claude Brisson:
> The Velocity Engine 2.0 RC5 is available.
>
> Main changes since the RC4:
>
> * the default encoding is now UTF-8 (and not the platform default)
> * commons-collections is not any more a compilation dependency
> * commons-lang3 dependency is not any more shaded
> * the configuration API doesn't use ExtProperties anymore
> * the events API has been reviewed: all events do receive the current
> Context as argument
>
> Release notes:
>
> *
> https://dist.apache.org/repos/dist/dev/velocity/velocity-engine/2.0/release-notes.html
>
>
> Distribution:
>
> * https://dist.apache.org/repos/dist/dev/velocity/velocity-engine/2.0/
>
> Maven 2 staging repository:
>
> *
> https://repository.apache.org/content/repositories/orgapachevelocity-1015
>
> If you have had a chance to review the test build, please respond with a
> vote on its quality:
>
> [ ] Leave at test build
> [ ] Alpha
> [ ] Beta
> [ ] General Availability (GA)
>
>
> Everyone who has tested the build is invited to vote. Votes by PMC
> members are considered binding. A vote passes if there are at least
> three binding +1s and more +1s than -1s.
Here are my comments referred in my the previous mail:
* README.txt requires a rewrite, it still talks about Ant, old directory
structures, and much more
* POM: use of JavaCC Maven Plugin: never ever add auto-generated code to
src/, it completely breaks
the agreed convention. Have it in target/ and attach the source code
with Buildhelper Maven Plugin, if
the JavaCC Plugin doesn't do it on its own.
* POM: The deletion/creation of the JavaCC parser should rather be a
complete profile rather than
a hack with properties passed to Ant, if this is necessary at all. I
see no difference here to code generation by JAXB's XJC or Modello, etc.
* There are several spots which
** don't use SLF4J's {} placeholder,
** don't use System.lineSeparator() but '\n'
** do a toLowerCase() (e.g., #sameEncoding()) instead of
toLowerCase(Locale.ROOT)
* There is a lot trailing whitespace throughout the code
* Generally, don't use log#isXXXEnabled() because of SLF4J's {}
placeholders' foo
* There are a few null argument checks which can be better handled with
Common Lang's Validate class
* StringBuffer is used sometimes where StringBuilder would suffice (w/o
synchronization)
* Stuff like BUILD_README.txt absolutely do not belong to src/main/java,
see second point
* There are some unused imports once in a while
* Rather use maven.compiler.source and maven.compiler.target properties
to avoid duplicate value setting, checkstyle, findbugs, JavaCC, etc.
* If a dependency is used several times, e.g. SFL4J, it shall be added
to the dep mngt section of the parent POM to avoid repetition of version
elements. Hint: this can be done with reactor modules too
There are likely to be more issues I have missed.
What is the benefit of velocity-engine-assembly actually? I know, ages
ago Velocity and other software was delivered as a tarball or zip file
with all JARs and the rest, including source. I think this is pretty
much obsolete now and partially redundant:
* Apache Parent already provides predefined descriptors which create a
source-release.zip for you, no further work necessary
** source-release.zip is used for release testing/votes, as well as for
clients/users who do not want to clone from Git or checkout via Subversion
* Binary artifacts are always available on Maven Central, I hardly
believe that someone will really download dist.zip to get the JARs and
add them manually somewhere, even if, they can get them from
search.maven.org
Michael
---------------------------------------------------------------------
To unsubscribe, e-mail: user-unsubscribe@velocity.apache.org
For additional commands, e-mail: user-help@velocity.apache.org
Re: [VOTE] Engine 2.0 RC5 Release quality
Posted by Greg Huber <gr...@gmail.com>.
[ ] Leave at test build
[ ] Alpha
[ ] Beta
[x] General Availability (GA)
+1 works great for me. Thanks.
On 16 January 2017 at 14:02, Claude Brisson <cl...@renegat.net> wrote:
> The Velocity Engine 2.0 RC5 is available.
>
> Main changes since the RC4:
>
> * the default encoding is now UTF-8 (and not the platform default)
> * commons-collections is not any more a compilation dependency
> * commons-lang3 dependency is not any more shaded
> * the configuration API doesn't use ExtProperties anymore
> * the events API has been reviewed: all events do receive the current
> Context as argument
>
> Release notes:
>
> * https://dist.apache.org/repos/dist/dev/velocity/velocity-eng
> ine/2.0/release-notes.html
>
> Distribution:
>
> * https://dist.apache.org/repos/dist/dev/velocity/velocity-engine/2.0/
>
> Maven 2 staging repository:
>
> * https://repository.apache.org/content/repositories/orgapache
> velocity-1015
>
> If you have had a chance to review the test build, please respond with a
> vote on its quality:
>
> [ ] Leave at test build
> [ ] Alpha
> [ ] Beta
> [ ] General Availability (GA)
>
>
> Everyone who has tested the build is invited to vote. Votes by PMC members
> are considered binding. A vote passes if there are at least three binding
> +1s and more +1s than -1s.
>
>
>
> Claude
>
>
>