Downloaded, checked hashes, built and ran tests on Ubuntu/JDK 14;
checked distro against git (see issue 1); reviewed release notes (see
issue 2).

+1 (binding) but issues 1 and 2 need to be fixed right after the release.

Issue 1. License file is not the same as in source control:

diff -r ./LICENSE /tmp/apache-calcite-1.24.0-src/LICENSE
177a178,189
>
> Additional License files can be found in the 'licenses' folder located in the 
> same directory as the LICENSE file (i.e. this file)
>
> - Software produced outside the ASF which is available under other licenses 
> (not Apache-2.0)
>
> MIT
> * cobyism:html5shiv:3.7.2
> * font-awesome:font-awesome-code:4.2.0
> * gridsim:gridsim:
> * jekyll:jekyll:
> * normalize:normalize:3.0.2
> * respond:respond:1.4.2

Can you fix the release instructions that the generated LICENSE needs
to be committed (probably at the same time you revise the release
notes).

Issue 2. Release notes

For the 'highlights', I prefer a paragraph with hyperlinks over a list
(see 
https://github.com/apache/calcite/blob/calcite-1.24.0-rc0/site/_docs/history.md#1180--2018-12-21).

Regarding categorization:
* why is 4032 'breaking'?
* why is 3786 breaking? (recomputeDigest was not present in 1.23; the
remarks about caching digests are useful, so why aren't they in the
javadoc?)
* we need a note that a bunch of methods are deprecated in this
release and will be removed before 1.25 (see 3923, 4023 and 4079).
This will break semantic versioning in 1.25, so is a big deal.
* 4073, 3224, 4056, 4008, 3972, 4060 are listed as new features, but I
think they are bug fixes or improved implementations
* 3946, 4089, 4087 are listed as fixes but could be listed as new features
* 4075 should be under 'test suite'
* 4094 description does not need 'follow-up after review comments'
* 4086 is an upgrade, so should be in 'bug fixes', not documentation
* A few places SQL and Java keywords are not in code font (e.g. NPE,
IllegalArgumentException, RexNode, Expression, HAVING, ARRAY, MAP,
CAST)

Julian

On Mon, Jul 20, 2020 at 12:01 PM Michael Mior <mm...@apache.org> wrote:
>
> +1
>
> Checked hash and signature and compiled and ran tests. Thanks Chunwei!
>
> --
> Michael Mior
> mm...@apache.org
>
> Le lun. 20 juil. 2020 à 11:41, Chunwei Lei <chunwei.l...@gmail.com> a écrit :
> >
> > Hi all,
> >
> > I have created a build for Apache Calcite 1.24.0, release
> > candidate 0.
> >
> > Thanks to everyone who has contributed to this release.
> >
> > You can read the release notes here:
> > https://github.com/apache/calcite/blob/calcite-1.24.0-rc0/site/_docs/history.md
> >
> > The commit to be voted upon:
> > https://gitbox.apache.org/repos/asf?p=calcite.git;a=commit;h=4b5b9100e59ae4a43424156c9beabec6805f3d7c
> >
> > Its hash is 4b5b9100e59ae4a43424156c9beabec6805f3d7c
> >
> > Tag:
> > https://github.com/apache/calcite/tree/calcite-1.24.0-rc0
> >
> > The artifacts to be voted on are located here:
> > https://dist.apache.org/repos/dist/dev/calcite/apache-calcite-1.24.0-rc0
> > (revision 40574)
> >
> > The hashes of the artifacts are as follows:
> > ffc7821089a444d50be228b0f0d9d8fb875c98f3b31ed0ad5a81cf5f56b9139dd353fd2c866b5bfd42a06c2a09bca579bcf6ed1e05322be1ae228fd7848f4aec
> > *apache-calcite-1.24.0-src.tar.gz
> >
> > A staged Maven repository is available for review at:
> > https://repository.apache.org/content/repositories/orgapachecalcite-1096/org/apache/calcite/
> >
> > Release artifacts are signed with the following key:
> > https://www.apache.org/dist/calcite/KEYS
> >
> > N.B.
> > To create the jars and test Apache Calcite: "./gradlew build".
> >
> > If you do not have a Java environment available, you can run the tests
> > using docker. To do so, install docker and docker-compose, then run
> > "docker-compose run test" from the root of the directory.
> >
> > Please vote on releasing this package as Apache Calcite 1.24.0.
> >
> > The vote is open for the next 72 hours and passes if a majority of at
> > least three +1 PMC votes are cast.
> >
> > [ ] +1 Release this package as Apache Calcite 1.24.0
> > [ ]  0 I don't feel strongly about it, but I'm okay with the release
> > [ ] -1 Do not release this package because...
> >
> >
> > Here is my vote:
> >
> > +1 (non-binding)
> >
> >
> >
> > Best,
> > Chunwei

Reply via email to