Add guava compatability up to 27.0.1#6948
Conversation
| * | ||
| * @return The result of invoking the method on the object | ||
| */ | ||
| private static <T> T tryMethods(Object object, Class<T> assignableTo, String... methods) |
There was a problem hiding this comment.
Documentation should reference the specific commit and the range of lines (as a Github link) in Guava from where this was copied. Same for other non-trivial parts of the copied code.
There was a problem hiding this comment.
This part was not copied. The method names in the callers are copied though. I'll add a javadoc here to help make sure a "use the git tag as a reference" pattern is followed in the future for such behaviors.
|
Keeping away stale bot, I still hope to have this one merged |
|
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@druid.apache.org list. Thank you for your contributions. |
|
go away stale bot! |
|
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@druid.apache.org list. Thank you for your contributions. |
|
This pull request/issue has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time. |
| } | ||
| } | ||
| if (matcher == null) { | ||
| throw new IllegalStateException("wtf!?"); |
There was a problem hiding this comment.
Please make the error message more constructive, or absent altogether
| } | ||
| }, | ||
| Execs.directExecutor() | ||
| Runnable::run |
There was a problem hiding this comment.
I think it's better to create a method like GuavaUtils.directExecutor() which returns Runnable::run. Bare Runnable::run looks somewhat puzzling. There are also other calls to Runnable::run in this PR which are subject for this.
| this.segmentWriteOutMediumFactory = segmentWriteOutMediumFactory; | ||
| } | ||
|
|
||
| private static String computeProcessingID(final String dataSource, final List<DataSegment> segments) |
There was a problem hiding this comment.
Could you please keep the old method arrangement so that it's visible what have changed in these methods?
| java.util.regex.Pattern#matches(java.lang.String,java.lang.CharSequence) @ Use String.startsWith(), endsWith(), contains(), or compile and cache a Pattern explicitly | ||
| org.apache.commons.io.FileUtils#getTempDirectory() @ Use org.junit.rules.TemporaryFolder for tests instead | ||
|
|
||
| com.google.common.net.HostAndPort#getHostText() @ Use org.apache.druid.common.guava.GuavaUtils#getHostText instead |
There was a problem hiding this comment.
Would be nice to break this file into sections: jackson, guava, jdk, everything else.
|
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@druid.apache.org list. Thank you for your contributions. |
|
This pull request/issue has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time. |
2 participants
Use reflection to ensure guava compatibility.
Runnable::runexecutor directly