Great review Alex, TY, I am canceling this vote and will reply to this email with details.
Gary On Tue, May 2, 2023, 12:14 Alex Herbert <alex.d.herb...@gmail.com> wrote: > Validated signatures and checksums. > > Build from source tar.gz file using 'mvn verify site' with: > > Apache Maven 3.9.0 (9b58d2bad23a66be161c4664ef21ce219c2c8584) > Maven home: /usr/local/apache-maven-3 > Java version: 11.0.18, vendor: Ubuntu, runtime: > /usr/lib/jvm/java-11-openjdk-amd64 > Default locale: en_GB, platform encoding: UTF-8 > OS name: "linux", version: "5.4.0-148-generic", arch: "amd64", family: > "unix" > > I do see the checkstyle and spotbugs report. > > Spotbugs has a few mentions of non-serializable fields. This is > another case where we may not wish to support serialization going > forward (e.g. as with CSV). I cannot remember the solution for CSV. I > think it was a formal documentation that serialization will be dropped > in CSV 2.0. Looking at the error these are not new as the report in > the live site for 2.11.0 also has these. > > Checked for the @since tag for new items in the japicmp report and > found no missing tags. I note that some of the new classes in package > io.build have no javadoc on inner classes with public constructors and > some static helper protected methods. > > New helper classes with a private constructor should be final: > > CharsetDecoders > CharsetEncoders > FilesUncheck > FileTimes > > Changing public classes to final after a release breaks binary > compatibility. However other utility classes in the lib do not use > final for classes so this is not critical. FYI there are 5 others > which I found using PMD. The rule is > ClassWithOnlyPrivateConstructorsShouldBeFinal [1]. This is not part of > the default rules used in the project which is using the default from > the maven plugin [2]. If you run 'mvn pmd:check' the rules are written > to target/pmd/rulesets. I took these rules, added the extra rule and > then used this as the ruleset for PMD. > > The new IOBiFunction has a noop method. This has no equivalent in > java.util.function.BiFunction. As discussed on a recent PR for Lang > (adding a noop to TriFunction) it does not make sense to have a noop > function that returns null as this is an operation. I assume this > BiFunction class was copied across from Lang bringing the noop with > it. > > A minor inconsistency: QueueInputStream uses the new > AbstractStreamBuilder API but existing public constructors are not > deprecated. However other classes with equally simple constructors > (MemoryMappedFileInputStream, MessageDigestCalculatingInputStream, > etc) have deprecated their public constructors in favour of the > builder. > > Documentation: The new AbstractStreamBuilder API exposes a lot of > public set methods in the builder. Some of these may not be applicable > to all use cases. The API allows an object for IO that is typically > created with either a Reader/Writer or Input/OutputStream, to also be > created with a Path or File. However creation using a Reader does not > support InputStream and vice versa. If a Reader is set then the Origin > will not be valid and a RTE will occur when opening the object which > tries to access the input stream from the Origin. Using the old API of > public constructors it was clear what the supported input arguments > were. All the Deprecated constructors that reference the new builder > would benefit from javadoc on the builder of the valid options that > can be configured. > > The new classes UncheckedBufferedReader, UncheckedFilterInputStream, > UncheckedFilterReader, UnsynchronizedBufferedInputStream, > UnsynchronizedFilterInputStream, UncheckedFilterOutputStream do not > use the AbstractStreamBuilder API and have constructors. I think the > API can be applied in these cases, although many of the options would > be ignored (e.g. charset, buffer size, etc). However there may be a > reason that blocks use of the API that I did not notice (as I did not > try to implement it). > > NullOutputStream deprecated constructor references deprecated > NULL_OUTPUT_STREAM singleton. Should be INSTANCE. Following on from > this should the NullPrintStream and NullWriter constructors also be > deprecated in favour of the INSTANCE? > > New class UncheckedFilterOutputStream has a public constructor but > UncheckedFilterWriter is protected. Both have a static 'on' method for > construction so this is inconsistent. These could use the > AbstractStreamBuilder API although the constructors are very simple. > Use of the new API would allow use of Path/File as output. > > New class ThreadUtils has a TODO in the javadoc. The javadoc also > requires <p> tags. The TODO can be moved to a method comment rather > than being within the javadoc where I do not think it is beneficial to > the end-user. > > I do not think these are blockers. Extra docs on the use of the > builders can be added later. But it may be nice to clean up little > inconsistencies in the new public before release. > > Alex > > [1] > https://pmd.github.io/pmd/pmd_rules_java_design.html#classwithonlyprivateconstructorsshouldbefinal > [2] > https://maven.apache.org/plugins/maven-pmd-plugin/pmd-mojo.html#rulesets > > On Tue, 2 May 2023 at 12:44, Gary Gregory <garydgreg...@gmail.com> wrote: > > > > Ping PMC (and others welcome). > > > > Gary > > > > > > On Mon, May 1, 2023, 08:47 Gary D. Gregory <ggreg...@apache.org> wrote: > > > > > Get we get more reviews, please? > > > > > > TY! > > > Gary > > > > > > On 2023/04/29 22:01:52 Bruno Kinoshita wrote: > > > > +1 > > > > > > > > Build from tag passed with no errors on > > > > > > > > Apache Maven 3.8.5 (3599d3414f046de2324203b78ddcf9b5e4388aa0) > > > > Maven home: /opt/apache-maven-3.8.5 > > > > Java version: 17.0.6, vendor: Private Build, runtime: > > > > /usr/lib/jvm/java-17-openjdk-amd64 > > > > Default locale: en_US, platform encoding: UTF-8 > > > > OS name: "linux", version: "5.15.0-70-generic", arch: "amd64", > family: > > > > "unix" > > > > > > > > Site reports look good. On my laptop it didn't create the spotbugs > and > > > > checkstyle reports with `mvn site`. But looking at the dist area site > > > > everything looks good. > > > > > > > > Thanks! > > > > > > > > Bruno > > > > > > > > > > > > On Sat, 29 Apr 2023 at 02:33, Gary Gregory <garydgreg...@gmail.com> > > > wrote: > > > > > > > > > We have fixed quite a few bugs and added some significant > enhancements > > > > > since Apache Commons IO 2.11.0 was released, so I would like to > > > > > release Apache Commons IO 2.12.0. > > > > > > > > > > Apache Commons IO 2.12.0 RC1 is available for review here: > > > > > https://dist.apache.org/repos/dist/dev/commons/io/2.12.0-RC1 > (svn > > > > > revision 61539) > > > > > > > > > > The Git tag commons-io-2.12.0-RC1 commit for this RC is > > > > > c780ef616bd6c7340f1d8a5dc8c209376a76451f which you can browse here: > > > > > > > > > > > > > > https://gitbox.apache.org/repos/asf?p=commons-io.git;a=commit;h=c780ef616bd6c7340f1d8a5dc8c209376a76451f > > > > > You may checkout this tag using: > > > > > git clone https://gitbox.apache.org/repos/asf/commons-io.git > > > > > --branch commons-io-2.12.0-RC1 commons-io-2.12.0-RC1 > > > > > > > > > > Maven artifacts are here: > > > > > > > > > > > > > > https://repository.apache.org/content/repositories/orgapachecommons-1633/commons-io/commons-io/2.12.0/ > > > > > > > > > > These are the artifacts and their hashes: > > > > > > > > > > #Release SHA-512s > > > > > #Fri Apr 28 17:58:50 EDT 2023 > > > > > > > > > > > > > > commons-io-2.12.0-bin.tar.gz=f291190a52b594300ba5df32ae2698378b56f999c1d6bd5391f277ff5e8c4f2deaf153e753e7377614949c9b69551b8897b0a6bad9f0a34f0c52d7a2c0b02344 > > > > > > > > > > > > > > commons-io-2.12.0-bin.zip=82f4cc82aa4e2b099afd552b7df21d7d882c7fa1ce7d1abef5023d361a1861ca62a50098caae7c905d7a077283ec618b2b81f2c446f66fd6c2bdd0df38d02700 > > > > > > > > > > > > > > commons-io-2.12.0-bom.json=44d65999a06397b2c22499d6488315f9dc40a7065547b922ca4d16ddc64d1679905a080cbd63151a7e7a0a68204b113139999778cc2cc980959cd2273d322a08 > > > > > > > > > > > > > > commons-io-2.12.0-bom.xml=2dec492e9758870eb6802c905bf7ce41a17b64d69722d1e1476600682d3859b1465cbcc9f6015702f7c9bfb3e581b122d11dad7054923b332d0ec4044c9612d4 > > > > > > > > > > > > > > commons-io-2.12.0-javadoc.jar=6ce2924eeca7fdaf3caceb0cd75df7ee584d3232c7b975a9b74442464bbd6da0dfb77132aabfdece2fa6e29a852255862f9a2add38b1c5969733667e248a509a > > > > > > > > > > > > > > commons-io-2.12.0-sources.jar=e31349d8480d6c8f3e91abc743076f683d6dabf1bebf594118bb15269680b92d98260264105d5ebe5d1e42bdb3efbe96426c575d70e0dd7731184e7e94b74d39 > > > > > > > > > > > > > > commons-io-2.12.0-src.tar.gz=f62d52cda73a42e1c63f339aa90d363ef4a37f378b49276b11ff7dcb5aae1a15aa8d4104daa97dd40d094d64b053566987495eb888162d166af7f61fe9a403b0 > > > > > > > > > > > > > > commons-io-2.12.0-src.zip=85538dbf9d57e381bd8a62e46caf0e5d97e18b4b960b3cdc5bb54d45dc420ad98ae9bae8d243f70bd609ea99d4e07e6188fac7ca4cb80a011f1331e46d03a6ce > > > > > > > > > > > > > > commons-io-2.12.0-test-sources.jar=31e7aa90d6f062a8f3fd9de5189f24878b080ea172d9286eb41e8c552b87bff4fc8202dd25e431ea5eb9ad7db9a8bb51471d680cb61003565426b506292277ff > > > > > > > > > > > > > > commons-io-2.12.0-tests.jar=054f2dac25b2afa352ce8f80a024d13cc49ab9a8bf3dff426698ba2b3b95c5d0092b906de8bc707a120f298069ed8f4b2154342a53d25038dcdfc3e2a3a1cc93 > > > > > > > > > > > > > > commons-io_commons-io-2.12.0.spdx.json=1375bad0d00979da1b86b304ca39ff52e1a693ed2a85ce7f29ba26734f5a71a837ebb57278727357f7450ede1ad3c671713dc708fbffdf19af381840cf1dafc3 > > > > > > > > > > I have tested this with 'mvn' (the default Maven goal) using: > > > > > > > > > > Apache Maven 3.9.1 (2e178502fcdbffc201671fb2537d0cb4b4cc58f8) > > > > > Maven home: /usr/local/Cellar/maven/3.9.1/libexec > > > > > Java version: 1.8.0_372, vendor: Homebrew, runtime: > > > > > /usr/local/Cellar/openjdk@8 > > > > > /1.8.0+372/libexec/openjdk.jdk/Contents/Home/jre > > > > > Default locale: en_US, platform encoding: UTF-8 > > > > > OS name: "mac os x", version: "13.3.1", arch: "x86_64", family: > "mac" > > > > > Darwin **** 22.4.0 Darwin Kernel Version 22.4.0: Mon Mar 6 > 21:00:17 > > > > > PST 2023; root:xnu-8796.101.5~3/RELEASE_X86_64 x86_64 > > > > > > > > > > Details of changes since 2.11.0 are in the release notes: > > > > > > > > > > > > > > https://dist.apache.org/repos/dist/dev/commons/io/2.12.0-RC1/RELEASE-NOTES.txt > > > > > > > > > > > > > > https://dist.apache.org/repos/dist/dev/commons/io/2.12.0-RC1/site/changes-report.html > > > > > > > > > > Site: > > > > > > > > > > > > > > https://dist.apache.org/repos/dist/dev/commons/io/2.12.0-RC1/site/index.html > > > > > (note some *relative* links are broken and the 2.12.0 > directories > > > > > are not yet created - these will be OK once the site is deployed.) > > > > > > > > > > JApiCmp Report (compared to 2.11.0): > > > > > > > > > > > > > > https://dist.apache.org/repos/dist/dev/commons/io/2.12.0-RC1/site/japicmp.html > > > > > > > > > > RAT Report: > > > > > > > > > > > > > > https://dist.apache.org/repos/dist/dev/commons/io/2.12.0-RC1/site/rat-report.html > > > > > > > > > > KEYS: > > > > > https://www.apache.org/dist/commons/KEYS > > > > > > > > > > Please review the release candidate and vote. > > > > > This vote will close no sooner than 72 hours from now. > > > > > > > > > > [ ] +1 Release these artifacts > > > > > [ ] +0 OK, but... > > > > > [ ] -0 OK, but really should fix... > > > > > [ ] -1 I oppose this release because... > > > > > > > > > > Thank you, > > > > > > > > > > Gary Gregory, > > > > > Release Manager (using key 86fdc7e2a11262cb) > > > > > > > > > > For following is intended as a helper and refresher for reviewers. > > > > > > > > > > Validating a release candidate > > > > > ============================== > > > > > > > > > > These guidelines are NOT complete. > > > > > > > > > > Requirements: Git, Java, Maven. > > > > > > > > > > You can validate a release from a release candidate (RC) tag as > > > follows. > > > > > > > > > > 1) Clone and checkout the RC tag > > > > > > > > > > git clone https://gitbox.apache.org/repos/asf/commons-io.git > --branch > > > > > commons-io-2.12.0-RC1 commons-io-2.12.0-RC1 > > > > > cd commons-io-2.12.0-RC1 > > > > > > > > > > 2) Check Apache licenses > > > > > > > > > > This step is not required if the site includes a RAT report page > which > > > > > you then must check. > > > > > > > > > > mvn apache-rat:check > > > > > > > > > > 3) Check binary compatibility > > > > > > > > > > Use JApiCmp with the japicmp Maven Profile: > > > > > > > > > > This step is not required if the site includes a JApiCmp report > page > > > > > which you then must check. > > > > > > > > > > mvn install -DskipTests -P japicmp japicmp:cmp > > > > > > > > > > 4) Build the package > > > > > > > > > > mvn -V clean package > > > > > > > > > > You can record the Maven and Java version produced by -V in your > VOTE > > > > > reply. > > > > > To gather OS information from a command line: > > > > > Windows: ver > > > > > Linux: uname -a > > > > > > > > > > 5) Build the site for a single module project > > > > > > > > > > Note: Some plugins require the components to be installed instead > of > > > > > packaged. > > > > > > > > > > mvn site > > > > > Check the site reports in: > > > > > - Windows: target\site\index.html > > > > > - Linux: target/site/index.html > > > > > > > > > > -the end- > > > > > > > > > > > --------------------------------------------------------------------- > > > > > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > > > > > For additional commands, e-mail: dev-h...@commons.apache.org > > > > > > > > > > > > > > > > > > > > --------------------------------------------------------------------- > > > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > > > For additional commands, e-mail: dev-h...@commons.apache.org > > > > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > >
