-1 from me unfortunately.

There are a lot of files in lib/ the bookkeeper-all which aren't
covered in the notice:
com.google.code.findbugs-jsr305-3.0.2.jar
com.google.errorprone-error_prone_annotations-2.1.2.jar
com.twitter-jsr166e-1.0.0.jar
com.twitter-libthrift-0.5.0-7.jar
com.twitter-libthrift-0.5.0-7.jar
com.twitter-scrooge-core_2.11-4.16.0.jar
com.twitter-twitter-server_2.11-1.29.0.jar
javax.inject-javax.inject-1.jar
javax.servlet-javax.servlet-api-3.1.0.jar

Bookkeeper-server notice doesn't cover:
com.google.code.findbugs-jsr305-3.0.2.jar
com.google.errorprone-error_prone_annotations-2.1.2.jar
javax.servlet-javax.servlet-api-3.1.0.jar

Don't use the lists above as a basis to fix though. Whoever is
updating should doublecheck that the NOTICE files cover everything in
lib. We're going to need a different NOTICE for bookkeeper-server and
bookkeeper-all also. It's probably worth getting maven to try and
generate these files for us.

Otherwise everything looks good.
1. checksums and signature checked out
2. findbugs, rat, and tests ran cleanly
3. Jepsen tests passed

The other thing that's needed for the next RC is that the breaks in
the API (around thrown exceptions), need to be noted clearly and
loudly in the release notes.

Best regards,
Ivan



On Wed, Dec 6, 2017 at 4:54 AM, Jia Zhai <zhaiji...@gmail.com> wrote:
> Thanks a lot Enrico for the verification, especially for the notes. Would
> you please also help open some issues on github to track your findings and
> suggestions?
>
> On Tue, Dec 5, 2017 at 10:54 PM, Enrico Olivelli <eolive...@gmail.com>
> wrote:
>
>> +1 (non binding)
>> looks good to me
>>
>> - Built and tested candidate source tar ball
>> - Run Bookie and basic Bookie shel commands from the "dist all" package
>> - Checked tag on GitHub
>> - All tests are passing on my downstream projects (some of them need
>> re-compiling or minor changes)
>>
>> Thank you Jia for driving this and to every body, I expect great
>> improvements in production
>>
>> Notes:
>>
>> 1) There is a failing test on my dev machine, even on master. I think this
>> is not blocker for the release. It must be some problem on my machine:
>>
>> testWithDiskFullAndAbilityToCreateNewIndexFile(org.apache.
>> bookkeeper.bookie.BookieInitializationTest)
>> Time elapsed: 12.871 sec  <<< FAILURE!
>> java.lang.AssertionError: Bookie should be up and running
>>     at
>> org.apache.bookkeeper.bookie.BookieInitializationTest.
>> testWithDiskFullAndAbilityToCreateNewIndexFile(
>> BookieInitializationTest.java:602)
>>
>>
>> 2) NOTICE reports very old copyright note (dates to 2015) -> we should
>> check this on every file, not just this one, it is not a problem I think
>>
>> 3) EnsemblePlacementPolicy changed signatures of methods -> compile time
>> issue on downstream projects, I already knew, not a problem. I will not
>> create any issue.
>>
>> 4) BookKeeper.Builder#build -> now throws BKException -> compile time issue
>> on downstream projects, but it is not a showstopper. I wlil not create any
>> issue. This was expected.
>>
>> 5) Dropped dependency on commons collections -> so this disappeared from
>> downstream projects -> it is not a real problem, downstream project must
>> explicitly declare their own dependencies, it is not a BK problem.
>>
>> 6) We have better "BKException#getMessage", this has some impact on test
>> cases of downstream projects -> it is not a problem, I consider this a bug
>> on downstream projects, testcases should be more robust as BK provides
>> typed Exceptions
>>
>> Enrico
>>
>>
>> 2017-12-05 6:19 GMT+01:00 Jia Zhai <zhai...@apache.org>:
>>
>> > Hi everyone,
>> >
>> > Please review and vote on the release candidate #0 for the version
>> > 4.6.0, as follows:
>> > [ ] +1, Approve the release
>> > [ ] -1, Do not approve the release (please provide specific comments)
>> >
>> > The complete staging area is available for your review, which includes:
>> > * Release notes [1]
>> > * The official Apache source and binary distributions to be deployed
>> > to dist.apache.org [2]
>> > * All artifacts to be deployed to the Maven Central Repository [3]
>> > * Source code tag "release-4.6.0" [4]
>> >
>> > BookKeeper's KEYS file contains PGP keys we used to sign this
>> > release:https://dist.apache.org/repos/dist/release/bookkeeper/KEYS
>> >
>> > Please download these packages and review this release candidate:
>> >
>> > - Review release notes
>> > - Download the source package (verify md5, shasum, and asc) and follow
>> the
>> > instructions to build and run the bookkeeper service.
>> > - Download the binary package (verify md5, shasum, and asc) and follow
>> the
>> > instructions to run the bookkeeper service.
>> > - Review maven repo, release tag, licenses, and any other things you
>> think
>> > it is important to a release.
>> >
>> > The vote will be open for at least 72 hours. It is adopted by majority
>> > approval, with at least 3 PMC affirmative votes.
>> >
>> > Thanks,
>> > Jia Zhai
>> >
>> > [1] *https://github.com/apache/bookkeeper/pull/759
>> > <https://github.com/apache/bookkeeper/pull/759>*
>> > [2] *https://dist.apache.org/repos/dist/dev/bookkeeper/
>> > bookkeeper-4.6.0-rc0/
>> > <https://dist.apache.org/repos/dist/dev/bookkeeper/bookkeeper-4.6.0-rc0/
>> >*
>> > [3] https://repository.apache.org/content/repositories/
>> > orgapachebookkeeper-1021/
>> > [4] https://github.com/apache/bookkeeper/tree/release-4.6.0
>> >
>>

Reply via email to