Hello Bertrand,
thanks for the review. Here is how I am addressing your comments:
> I have reviewed SHA1(incubating-netbeans-html4j-1.5.zip)=
> eaa3a5a784f56fd1a20674ef5b59eed2be99d2fe
>
> I'm -1 on the release due to the missing DISCLAIMER which is easy to
> fix, details below.
>
> Once that's fixed, if others (including at least one other mentor)
> agree I'd be ok to promote the fixed release to Incubator PMC vote
> except for one thing: I suggest including in the Incubator PMC vote
> message instructions for reviewing the license/copyright header
> changes done since the code was imported from the Oracle donation, can
> you provide those? Probably just a git diff command or URL. A
> reference (git tag?) to tools used for these changes is also useful.
I made the change in two steps:
https://github.com/apache/incubator-netbeans-html4j/commit/a262480a0126b67975389685925bf5c3e13b4061
and then, after review by Jan Lahoda, I've also added:
https://github.com/apache/incubator-netbeans-html4j/commit/b4fdfc8314c8073059e91de4eea586d194a6b76f
Then the change has been reviewed by Craig.
We used a wikipage to track our steps:
https://cwiki.apache.org/confluence/display/NETBEANS/NetBeans+Transition+Process
- see the html4j columns.
>
> The digests are ok.
>
> I cannot verify the signature as I haven't found your 7E654BAC public
> key online, see the "PMC members please add your PGP keys to
> https://id.apache.org" thread here.
I'll work on fixing this today.
>
> <jaroslav.tulach@gmail.com> wrote:
> > ...The incubating-netbeans-html4j-1.5.zip.asc isn't present on the
server
> > (as I would need to upload my private key there),...
>
> IIUC this is because the release is generated by Jenkins and you don't
> want it to have your private key - sounds reasonable ;-)
>
> In this case maybe it's more convenient to keep the .asc file in the
> gi tools repository before the release happens?
> This also allows others to add their own signature if desired.
I plan to use the same technique I used to sign build #17, I just make sure
the key is properly registered.
> zip archive review:
>
> 1) DISCLAIMER is missing, for a podling this is enough to reject the
> release. See also DEPENDENCIES below, the same mechanism can be used
> to generate both.
Both of these files are now added to the repository:
https://github.com/apache/incubator-netbeans-html4j/commit/9dd7a4c2bdf207c81e6cf692f12689c19efebd02
> 2) In the NOTICE, I find this confusing....
> ...I would prefer
>
> "The initial code is based on NetBeans HTML/Java API modules kindly
> donated by Oracle Corp to the Apache Software Foundation"
Changed as you suggested:
https://github.com/apache/incubator-netbeans-html4j/commit/8952acf173e0a0cd6783867246c030d775820491
>
> 3) Having a DEPENDENCIES file would make it much easier to validate
> third-party dependencies, see this recent thread:
>
>
https://lists.apache.org/thread.html/b82a116ec77639c49528d654a488ff354ca8edf
> b8f4f380865762275@%3Cdev.netbeans.apache.org%3E
>
> Right now, " mvn dependency:tree" lists some unfamiliar dependencies,
> having their license in DEPENDENCIES would help validate them.
I've added the plugin as suggested
https://github.com/apache/incubator-netbeans-html4j/commit/9dd7a4c2bdf207c81e6cf692f12689c19efebd02#diff-600376dffeb79835ede4a0b285078036
and copied the DEPENDENCIES and DISCLAIMER files into root of the git
repository.
> 4) A quick build with "mvn clean install -DskipTests" works including
> apache-rat-plugin:0.12:check .
Good.
> The tests take a long time to run, I
> haven't checked if there's a way to run just unit tests.
The build with tests [takes few minutes](
https://builds.apache.org/job/incubator-netbeans-html4j-linux/8/). It is
using JavaFX WebView behind the scene and there may be various issues with
it. If the build gets stalled, feel free to generate jstack thread dump and
report me a bug. However that is just a functional problem, nothing to
prevent an incubating release, I assume.
>
> 5) the apache-rat-plugin excludes look good to me:
OK.
> 6) The release includes source code only, apart from a few .png
> (test?) images which are fine.
./geo/src/main/java/net/java/html/geo/doc-files/GeoDuke.png
./json/src/main/java/net/java/html/json/doc-files/html4j.png
./json/src/main/java/net/java/html/json/doc-files/DukeHTML.png
The above images are used in Javadoc. The images below are used in UI of
WebView Debugging Chrome:
./boot-fx/src/main/resources/org/netbeans/html/boot/fx/desktop.png
./boot-fx/src/main/resources/org/netbeans/html/boot/fx/handheldPortrait.png
./boot-fx/src/main/resources/org/netbeans/html/boot/fx/tabletPortrait.png
./boot-fx/src/main/resources/org/netbeans/html/boot/fx/netbook.png
./boot-fx/src/main/resources/org/netbeans/html/boot/fx/widescreen.png
./boot-fx/src/main/resources/org/netbeans/html/boot/fx/handheldLandscape.png
./boot-fx/src/main/resources/org/netbeans/html/boot/fx/selectionMode.png
./boot-fx/src/main/resources/org/netbeans/html/boot/fx/tabletLandscape.png
./boot-fx/src/main/resources/org/netbeans/html/boot/fx/sizeToFit.png
I am not going to call a new vote, but here is build #19, that shall
hopefully be closer to the desired state:
https://builds.apache.org/job/incubator-netbeans-html4j-release/19/
I'll work on signing it with proper key today. Let me know if something
else shall be addressed. Thanks in advance for your help.
-jt
|