Build and restli-tools-scala - first scan

[FranklinYinanDing]

• Structure-wise, I prefer all projects on the same level, rather than a top level src/ and also restli-tools-scala.
• pegasusVersion doesn't need to be an SBT setting, could be just a val?
• Talked offline, adding pegasus for consumer isn't ideal, we'd better let consumer add dependency by themselves and do version compatibility check.
• this dependency seems to be redundant, as you are already having inter-module dependency.
• this special dependency could be separated and add some comment for documentation.
• root could also be lazy? And a better project val name than root?
• log4j version could be exacted out as a val. Same for specs2
• What's the intention of this
• Doc s Provider?
• nitpick and elsewhere too scaladoc style instead?
• not very Scala style, instead, you can have a default value in the constrctor above. Unless you are thinking this could be used in Java.
• Is this an implementation? If so, it is better to add override keyword. Outside IDE, it is hard to differentiate. Elsewhere too. And also should inherit super's doc, unless you did something special here.
• Usually, we try to avoid state or var. Is this necessary for the workflow to work? And looks like it is always None?
• error => log.error(error) could be log.error(_).
• this could be simplified as settings.usejavacp.value = (classpath == null) || classpath.mkString(":") or settings.usejavacp.value = Option(classpath).map(_.mkString(":")).getOrElse(true).
• Is this because Scala doesn't usually use @Deprecate?
• This could combine with the above line.
• Could remove this if not documented or you can fill in something. Same elsewhere.
• Here also could be one-liner.

[TylerHorth]

• What's the intention of this

Scripted is the test runner for sbt tests. Scripted tests work by first publishing the plugin locally, then running scripts against each project under the sbt-test directory. This code passes the current plugin version to each test, which is used in the tests project/plugins.sbt file to add the locally published sb-restli dependency.

• not very Scala style, instead, you can have a default value in the constrctor above. Unless you are thinking this could be used in Java.

It is used from restli. I haven't made any changes to the ScalaDocsProvider besides what was needed to get it building on scala 2.12. I'll have to look into compatibility requirements for java-scala binding, but it looks like only the DocsProvider interface and a no arguments constructor is needed.

• Usually, we try to avoid state or var. Is this necessary for the workflow to work? And looks like it is always None?

This is set when registerSourceFiles is called (part of the DocsProvider interface), and used by the findTemplate method.

• this could be simplified as settings.usejavacp.value = (classpath == null) || classpath.mkString(":") or settings.usejavacp.value = Option(classpath).map(_.mkString(":")).getOrElse(true).

It's easy to miss, but this isn't actually equivalent:
settings.classpath is a string, and should be generated from the classpath array if it's not null.
settings.usejavacp is a boolean, and should be set to true if classpath is null.

I've made the changes to build.sbt, and I'm working on the other ScalaDocsProvider changes now.

[TylerHorth]

@FranklinYinanDing I think I've addressed everything. Let me know if I can close this issue.

[FranklinYinanDing]

Thanks for working on it!