feat: introduce java.time variables and methods - #3495
Conversation
product-auto-label
Bot
added
the
api: spanner
diegomarquezp
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
| private static void maybeWaitForSessionCreation( | ||
| SessionPoolOptions sessionPoolOptions, ApiFuture<SessionReference> future) { | ||
| org.threeten.bp.Duration waitDuration = sessionPoolOptions.getWaitForMinSessions(); | ||
| java.time.Duration waitDuration = sessionPoolOptions.getWaitForMinSessions(); |
There was a problem hiding this comment.
nit: java.time.Duration is already imported in this class. Can't we just remove org.threeten.bp.
|
|
||
| @Override | ||
| public void attemptFailed(Throwable error, Duration delay) { | ||
| public void attemptFailed(Throwable error, org.threeten.bp.Duration delay) { |
There was a problem hiding this comment.
nit: Can we import java.time.Duration so that we can remove java.time. in attemptFailedDuration method? https://github.com/googleapis/java-spanner/blob/main/google-cloud-spanner/src/main/java/com/google/cloud/spanner/CompositeTracer.java#L119
|
|
||
| @Override | ||
| public void attemptFailed(Throwable error, Duration delay) { | ||
| public void attemptFailedDuration(Throwable error, java.time.Duration delay) { |
There was a problem hiding this comment.
nit: Can we import java.time.Duration?
| private final boolean autoDetectDialect; | ||
| private final Duration waitForMinSessions; | ||
| private final Duration acquireSessionTimeout; | ||
| private final java.time.Duration waitForMinSessions; |
There was a problem hiding this comment.
nit: We can import java.time.Duration so that in future we don't have a confusion on which class to use
There was a problem hiding this comment.
and also it avoids so many changes in the code which is not really necessary.
|
@diegomarquezp Can we make sure following in all the files?
|
@sakthivelmanii Thanks for the review! Before switching to Promoting |
| } | ||
|
|
||
| org.threeten.bp.Duration asDuration() { | ||
| java.time.Duration asDuration() { |
There was a problem hiding this comment.
need to keep this one in favor of com.google.protobuf.Duration
diegomarquezp
added
the
automerge
gcf-merge-on-green
Bot
removed
the
automerge
5 participants
This PR introduces
java.timealternatives to existingorg.threeten.bp.*methods, as well as switching internal variables (if any) tojava.timeThe main constraint is to keep the changes backwards compatible, so for each existing threeten method "
method1(org.threeten.bp.Duration)" we will add an alternative with a Duration (or Timestamp when applicable) suffix: "method1Duration(java.time.Duration)".For most cases, the implementation will be held in the
java.timemethod and the old threeten method will just delegate the call to it. However, for the case of abstract classes, the implementation will be kept in the threeten method to avoid breaking changes (i.e. users that already overloaded the method in their user code).