Skip to content
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

Build for Java 8 rather than Java 7. #2744

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

eamonnmcmanus
Copy link
Member

@eamonnmcmanus eamonnmcmanus commented Sep 18, 2024

Closes #2743.

@eamonnmcmanus eamonnmcmanus added the java8 Issues related to making Java 8 the minimum supported version label Sep 18, 2024
Copy link
Member

@cpovirk cpovirk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few notes, mostly, on the Android front:

  • I see that you already have an Animal Sniffer check for Android compatibility. We seem to be getting good results in Guava from the https://github.com/open-toast/gummy-bears signatures that you mention, the ones that assume only the always-on desugaring of a smallish set of methods. Those are safe to use even if your users don't opt in to the more extensive feature that we normally refer to as "library desugaring," so you might have a look if javac starts generating usages of those methods automatically(?) or if you just want a smattering of Java 8 goodies.
  • I don't see any issues with conditional support for Optional and Instant and friends. Once you've checked that the classes exist (see sub-bullet), you'll now be able to write normal Java code to operate on those classes. Android doesn't even mind if you have random methods in a class with Optional and Instant in the signature (in contrast to Java, which would, IIUC) as long as you don't call them.
    • I imagine the best way to guard that support will be with Java reflection that looks up those classes. I would hope that that would support both the case of Android versions new enough to support the classes directly and the case of desugaring (which I hope is smart enough to rewrite strings used in reflection). I have made the mistake before of just trying to use the classes and then catching the resulting error, but I have learned one or two (see also internal thread "Android and java7 Futures.getChecked()" about log spam) lessons about that, at least one of which is unlikely to be news to you :)).
  • I have seen a handful of problems when experimenting with adding default methods in Guava inside Google. (See bug 229266760.) One of them in a result of a system that ill-advisedly tries to use a mix of the internal Guava and the external Guava (with the external Guava still lacking the default method), but I'm not sure I ever had a theory for the other. Anyway, adding the default methods would probably work fine, but I'd advise factoring in at least a little possibility that it turns out not to.
  • Lambda and method references should be entirely fine :) Maybe also remove some unnecessary final keywords from effectively final variables if you have any of those hanging around.
  • Kudos on actually using --release to avoid ByteBuffer issues (not that GSON appears to use the class) and other horrors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
java8 Issues related to making Java 8 the minimum supported version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make Java 8 the minimum supported version
2 participants