Looks fine to me.
Let's resync after this push and see if there is some low hanging fruit
we can pick, like the areas with 1 warning?
-Chris.
On 28/12/2011 01:57, Stuart Marks wrote:
Hi all, here's any updated webrev for this fix:
http://cr.openjdk.java.net/~smarks/reviews/7122061/webrev.1/
Changes from the previous webrev:
* I've withdrawn the addition of -Werror from several Makefiles. There
are two reasons. First, some of these had a single warning, causing
javac to emit the message "1 warning" that was missed by my analysis
script which was looking for the string "warnings". Second, some of the
steps had zero warnings but the addition of closed sources caused
warnings to appear. As a result I won't be adding -Werror to the following:
make/com/sun/servicetag/Makefile
make/java/zip/Makefile
make/javax/management/Makefile
make/jpda/bdi/Makefile
make/sun/pisces/Makefile
make/sun/tracing/Makefile
* The webrev shows no apparent changes to make/javax/security/Makefile.
Interesting story here. I saw that this build step had no warnings, so I
went to add JAVAC_MAX_WARNINGS=true JAVAC_WARNINGS_FATAL=true to the
Makefile, and I was surprised to discover these declarations already
present! Well, they were present but with trailing spaces. Turns out
that trailing spaces are significant in the way that these make
variables are used :-( which caused them not to be honored. (Make
strikes again!) Removing the trailing spaces enables the intended behavior.
**
With these changes, 52 out of 93 build steps are warnings-free and have
-Werror. I do think at some point we should look at having -Werror on by
default, as David Holmes suggested [1], but some additional cleanup in
this area is necessary and I want to coordinate with the build
infrastructure work to see how much effort is appropriate here.
Meanwhile I'd prefer to continue in a piecemeal approach.
Thanks.
s'marks
[1]
http://mail.openjdk.java.net/pipermail/build-dev/2011-December/005301.html
