Doc, Stock and Two Smoking HTML Tags: Cleanup pass in lift-common scaladocs #1650
Conversation
We do two things: turn on autoAPIMappings, which automatically lets us link to Scaladocs for modules that set their public URIs, and explicitly add the mapping for the scala-xml module. The addition of the scala-xml module is also reasonably generic, so we can add more module-specific URIs fairly easily in the future as needed.
Some places get more detailed Scaladocs, others get some facelifts and get converted to wiki syntax instead of HTML. Examples added throughout.
Some of the scaladocs were extremely redundant with respect to the functions they were describing. This commit drops those Scaladocs.
It doesn’t seem to be used anywhere and we generally lean on transformation functions rather than generation functions for NodeSeq. This is also something we want to continue to encourage, so making using NodeSeqs directly a little more painful is okay.
checkConfig was used to make sure setup was being run. It’s now named ranSetup, since that’s more reflective of what it means.
SimpleList and SimpleVector have a bunch of unsupported operations with respect to regular Java collections; these were through Exception instances. We switch them to throw UnsupportedOperationException as per the Java collection contract.
| * Implements an LRU Hashmap. Given a size, this map will evict the least | ||
| * recently used item(s) when new items are added. | ||
| * | ||
| * Note that `LRUMap` is *not* thread-safe. |
There was a problem hiding this comment.
The emphasised "not" renders in HTML as *not*. Looks like the scaladoc markup needs ''not'' for italic if that's the intention (https://wiki.scala-lang.org/display/SW/Syntax)
There was a problem hiding this comment.
Bah, yeah, good catch, I forgot to double-check those even though I meant to. Just use Common Mark dammit ;)
|
Nice! Thank you for tackling this. |
| * Empty otherwise | ||
| * Create a `Box` from the specified `Box`, checking for `null`. | ||
| * | ||
| * @return `Full(in)` if `in` is non-null, `Empty` otherwise. |
There was a problem hiding this comment.
Meh, this guy needs further clarification.
|
I would say that while some of the comments you removed are pretty redundant, when users are new to Scala and/or Lift, they actually help explain what those two or three lines do. I know I learned a lot by reading the source code and its comments. |
|
Any chance you can comment on the specific ones you think we shouldn't lose? I can see the argument for some, for example the implicit conversions that in turn take implicit parameters, but not so much for others, for example the case class descriptions that basically repeat the case class name. |
|
sure, I'll comment on the those |
| * @param justification Justify why calling this method is okay and why it will not result in an Exception | ||
| * | ||
| * @return The contents of the Box if it has one or an exception if not | ||
| */ |
There was a problem hiding this comment.
I would leave this one in, I have spoken to a few developers who thought using openOrThrowException was not a big deal, without really understanding that they are not really supposed to use it, (there are use cases for it, but it would be good to point out the alternatives)
There was a problem hiding this comment.
This actually got removed because it should be inherited from Box, which has the same comment above.
|
done with comments about what to kepp |
Apparently * doesn’t emphasize, ''' does. Seems legit…
Mostly adding `` around classes and parameters in docs, with a couple of tweaks to docs that weren’t clear enough.
The specific motivation is that scaladoc in 2.11.3 fixes an issue with picking up indentation in code blocks properly.
These are implicits that take implicits as parameters, and the docs make it a little clearer what that’s for.
|
Pushed fixes to most of the comments, including my own. Also clarified and tweaked some of the docs I'd written that clearly suffered from you-wrote-this-too-late-in-the-evening syndrome. Also, bumped to Scala 2.11.3 as it fixes an issue with recognizing indentation properly in scaladoc code blocks. Lastly, I have a proposed fix to the |
|
Thanks for the changes! |
|
Well, 'tis the season after all! ;) |
|
|
||
| crossScalaVersions in ThisBuild := Seq("2.11.2") | ||
| crossScalaVersions in ThisBuild := Seq("2.11.3") |
There was a problem hiding this comment.
Scala 2.11.3 was DOA. It broke binary compatibility. We need to bump directly to 2.11.4 (which is already out anyway)
Source: https://groups.google.com/forum/#!topic/scala-internals/SSD9BNJaFbU
Evidently 2.11.3 had binary compatibility issues, so 2.11.4 it is!
|
Added the 2.11.4 bump for maximum binary neighborly compatibility. |
|
Any thoughts on the fixes for |
|
looks good to me, 👍 |
|
Kk, pushing accordingly then. Will wait a couple of days for further comments before merging. |
|
Kk, seeing no further comments I'm going to go ahead and merge this feller in! |
Doc, Stock and Two Smoking HTML Tags: Cleanup pass in lift-common scaladocs This pass removes HTML from scaladocs, adds wiki markup in its stead, adds wiki markup in places where we were using just text, adds examples here and there, and generally tidies up the documentation. It also adds the ability to link to Scala's APIs and scala-xml APIs from our scaladocs, which required some tweaks to the build.
Hopefully I'll get to do this with the other modules in short order as well. This pass
removes HTML from scaladocs, adds wiki markup in its stead, adds wiki markup
in places where we were using just text, adds examples here and there, and generally
tidies up the documentation.
It also adds the ability to link to Scala's APIs and
scala-xmlAPIs from our scaladocs,which required some tweaks to the build.
In the process, also did some very slight code cleanup. Each of these is split out into
a commit in case we want to selectively remove it. Of particular interest is that I
removed documentation that seemed to add nothing on top of the existing function
name or signature in 55fa831. Let me know if you think that's a problem.