dsmiley commented on code in PR #3173:
URL: https://github.com/apache/solr/pull/3173#discussion_r1949280001
##
solr/core/src/java/org/apache/solr/util/TimeOut.java:
##
@@ -23,15 +23,31 @@
import java.util.function.Supplier;
import org.apache.solr.common.util.TimeSource;
+/**
+ * Timeout tool to ease time unit conversion, to check time left or time
elapsed, and to easily wait
Review Comment:
Thanks for adding javadocs! IMO the very first thing and only thing to list
is `check time left or time elapsed` as this is the fundamental limited
purpose. You wouldn't use this class only for time unit conversion :-).
Everything else is minor stuff that hopefully makes this utility easy to
use/understand.
##
solr/core/src/java/org/apache/solr/util/TimeOut.java:
##
@@ -23,15 +23,31 @@
import java.util.function.Supplier;
import org.apache.solr.common.util.TimeSource;
+/**
+ * Timeout tool to ease time unit conversion, to check time left or time
elapsed, and to easily wait
+ * on a {@link Supplier}. It is based on {@link TimeSource} to allow choosing
the time source, and
+ * to facilitate time control in tests.
+ */
public class TimeOut {
+ // Internally, the time unit is nanosecond.
private final long timeoutAt, startTime;
private final TimeSource timeSource;
+ /**
+ * Internally the timeout is stored in nanoseconds, so it cannot track more
than {@link
Review Comment:
The javadocs for anything should not begin with discussing internals. What
you added is great but it's an FYI. And even the FYI should start with that
great warning on what can happen (a limitation of sorts), and _then_ elaborate
with the _why_ (you started with the _why_) or leave the "why" (which is
distracting to the caller; they don't care) as code comment. As I look at this
with fresh eyes, I would have loved to see an explanation of what "interval"
is which I can eventually guess but the name choice wasn't obvious to me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
