TylerHorth/sbt-restli

sbt-restli first scan

Closed this issue · 2 comments

  • What's the log4j2.xml for?
  • Could this just be sbt._?
  • Instead of restliAvroGenerate, I prefer verb-first generateRestliAvro for tasks. Same elsewhere.
  • Why here needs mkdir where avro-plugin doesn't?
  • Should these be configurable?
  • Default to backwards?
  • Here no need to also watch?
  • How large is the summary? If too much, then it might be too noisy to be on info level.
  • Here needs some more comment to explain.
  • Why Throwable?
  • What's the log4j2.xml for?

Sbt 1.0 includes log4j and does configuration programatically, which will ignore the configuration from log4j2.xml. For sbt 0.13 and for forked jvm tasks (restliModelGenerate), we must provide log4j configuration ourselves, which comes from log4j2.xml.

  • Instead of restliAvroGenerate, I prefer verb-first generateRestliAvro for tasks. Same elsewhere.

Prefixing plugin tasks and keys with the plugin name is a sbt best practice. We can subvert this if you think it's a significant ergonomic improvement, but I think there's value in conformity.

  • Why here needs mkdir where avro-plugin doesn't?

The restli tools (AvroSchemaGenerator, PegasusDataTemplateGenerator, etc.) create the directories for us already. It's only RestRequestBuilderGenerator which doesn't.

  • Should these be configurable?

I'm not sure if there's much value in it. The version and deprecatedByVersion parameters are only useful if you want to generate deprecated client bindings (you have to cut deprecated features at some point, right?). The other two parameters, generateImported and generateDataTemplates, I couldn't think of a use case for, and the gradle plugin doesn't let you configure them either. generateDataTemplates is already covered by the RestliSchemaPlugin, and generateImported generates client bindings for imported projects, when you could just depend on the client bindings of said project.

  • Here no need to also watch?

Since rest models are generated from java/scala source files, they are already being watched.

  • How large is the summary? If too much, then it might be too noisy to be on info level.

The summary prints out the list of compatible changes (or incompatible), and is meant to be user facing. It only prints the first time a change is made, so it shouldn't be too noisy.

When exceptions go uncaught and do not implement the FeedbackProvidedException, sbt prints out the whole exception (including stacktrace) twice. This is of course pretty noisy for exceptions like malformed json or unresolved schema URIs. I'll see if I can narrow this down to a few more specific exceptions, so other errors still get a full stacktrace.

Thanks for the explanation!