Conversation
| @inline def compareAndSet(ll: LazyListBase[?], expected: AnyRef, value: AnyRef): Boolean = | ||
| if (ll._tail eq expected) { ll._tail = value; true } else false | ||
|
|
||
| /** Sets the `_tail` field of `ll` to `value`. |
There was a problem hiding this comment.
Do we really want to document internal implementation details in public documentation?
Also all package-private definitions are not user facing, maybe we don't need to explicitlly document them?
There was a problem hiding this comment.
Good question. About a quarter of the added docs in these PRs are non-user facing, so that would be for maintainers alone. I can strip that out if people think it is more clutter than clarity. I'm not sure if there's a compile time cost, but there could be a comprehension cost if the bare, uncommented code is easier for maintainers to work with. I'm curious to hear what people think.
b4980d4 to
86f1335
Compare
36d8b8c to
38121c0
Compare
cheeseng
left a comment
cheeseng
left a comment
There was a problem hiding this comment.
Added some comments about the new scaladocs.
| /** Returns a message describing the object that failed to match. | ||
| * | ||
| * The message contains the object's string representation and its class name, | ||
| * falling back to the class name alone if its `toString` throws, and naming |
There was a problem hiding this comment.
The fallback, when happens, is:
"an instance " + ofClass
Not just the class name alone.
Also, the comment didn't mention that when obj is null it will return "null".
| @@ -40,9 +40,15 @@ import scala.collection.{AbstractIterator, AnyStepper, IterableFactoryDefaults, | |||
| */ | |||
| @SerialVersionUID(3L) | |||
| sealed class NumericRange[T]( | |||
There was a problem hiding this comment.
Not sure if it is due to the 'sealed' keyword that makes our tool to not process this, the current scaladoc does not have @tparam fot T.
| trait NoStackTrace extends Throwable { | ||
| /** Overrides the default stack trace filling behavior to optionally suppress stack traces for efficiency. | ||
| * | ||
| * @return this `Throwable` instance without filling in the stack trace if suppression is enabled, otherwise the result of the superclass implementation |
There was a problem hiding this comment.
May be simpler with:
"returns this unchanged when stack traces are suppressed, otherwise fills in the stack trace".
| * to `Array[AnyRef]`. Arrays are covariant in their | ||
| * element type at run time. | ||
| * | ||
| * `Array.equals(xs.asInstanceOf[Array[AnyRef]], ys.asInstanceOf[Array[AnyRef]])` |
There was a problem hiding this comment.
Better to use triple quotes here.
3 participants
This PR fills in a main doc comment plus @PARAM, @tparam, and @return tags for
scala.collection.convertand the Scala.js variants of the standard library that are completely missing any Scaladoc documentation. Most of it is the stepper implementations undercollection.convert.impl; the rest is the Scala.js counterparts of files whose JVM versions are documented in the other pull requests in this series, acrossscala,scala.collection.immutable,scala.collection.mutable,scala.runtime,scala.reflect,scala.util,scala.math,scala.concurrentandscala.scalajs. I'm submitting it as a draft PR so that I can get the CI to run on it, to see if it breaks anything, and to start getting feedback. We automated the generation of these changes and have not reviewed all of them yet. We will review them all before making the PR non-draft. Please let me know whether you think this is going in the right direction in general, and anything specific that you notice that could be improved.