-
Notifications
You must be signed in to change notification settings - Fork 10.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Javadoc search on JDK 11: #5800
Conversation
Failure under JDK8, which we run CI under. As expected:
Maybe just add...
...to... guava/.github/workflows/ci.yml Line 43 in e64ad26
...given that "actual" Javadoc generation (for snapshots) is handled by... guava/.github/workflows/ci.yml Line 83 in e64ad26
|
Hmm, but then pre-commit CI won't check Javadoc generation, so we wouldn't hear about breakages until after the commit. Maybe it makes more sense for the flag to be conditional on the Java version -- either in the CI config or in Maven, as I think you'd suggested. @cgdecker in case he happens to have thought about this. |
...not sure offhand why Truth doesn't show this same problem.... |
Sorry, I'm now going back through some of your earlier messages. Arguably an even better way to approach this would be to always build with some new version of javac but then test with each version we're targeting. My guess, though, is that that's harder. |
...but I guess it's nice for Guava users to be able to build Guava with different JDKs. |
Thanks for taking a look! Oh, I was convinced we skip javadocs by default, but I see it makes sense to run them after the integration tests if we also publish them on each commit. I agree that punishing 17+ -using users and requiring a 3y-old JDK is not good :) What do you think would be the best thing to do:
|
Thanks. (3) has some chance of turning into a bunch of work, which I predict that I personally will not follow up on. So, between (1) and (2), I think you make a good case that (1) is nicer. |
Fixed Javadoc search feature on JDK 11, which is currently used in our scripts updating snapshot Javadocs on guava.dev and building Guava releases, by adding `no-module-directories` flag. This option is not present in javadoc tool from JDK 8 and 13+, hence we use profiles to conditionally pass this flag on 9-12 only. Note that on JDK 17 javadoc generation does not work, as it seems to have changed the warning on LVTI usage in sources to an error. Fixes google#5457 TESTED=Locally with `mvn javadoc:javadoc` for `guava` module and its android flavour
29639a1
to
7d1c2ff
Compare
Changed to (1) and updated the reasoning in the description. However, on JDK 17 it doesn't work just yet: #5801 (comment) |
Fixed Javadoc search feature on JDK 11, which is currently used in our scripts updating snapshot Javadocs on guava.dev and building Guava releases, by adding `no-module-directories` flag. This option is not present in javadoc tool from JDK 8 and 13+, hence we use profiles to conditionally pass this flag on 9-12 only. Note that on JDK 17 javadoc generation does not work, as it seems to have changed the warning on LVTI usage in sources to an error. Fixes #5457 Fixes #5800 RELNOTES=n/a PiperOrigin-RevId: 413922237
Fixed Javadoc search feature on JDK 11, which is currently used in our scripts updating snapshot Javadocs on guava.dev and building Guava releases, by adding `no-module-directories` flag. This option is not present in javadoc tool from JDK 8 and 13+, hence we use profiles to conditionally pass this flag on 9-12 only. Note that on JDK 17 javadoc generation does not work, as it seems to have changed the warning on LVTI usage in sources to an error. Fixes #5457 Fixes #5800 RELNOTES=n/a PiperOrigin-RevId: 413922237
Fixed Javadoc search feature on JDK 11, which is currently used in our
scripts updating snapshot Javadocs on guava.dev and building
Guava releases, by adding
no-module-directories
flag.This option is not present in javadoc tool from JDK 8 and 13+,
hence we use profiles to conditionally pass this flag on 9-12 only.
Note that on JDK 17 javadoc generation does not work,
as it seems to have changed the warning on LVTI usage in sources
to an error.
Fixes #5457
TESTED=Locally with
mvn javadoc:javadoc
forguava
module and its android flavour on JDK 11