Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docs #1

Merged
merged 18 commits into from Jul 31, 2018
Merged

Docs #1

merged 18 commits into from Jul 31, 2018

Conversation

pepegar
Copy link
Member

@pepegar pepegar commented Jul 31, 2018

This PR:

  • adds some docs
  • adds test for the Avro conversions.
  • adds sbt-freestyle
  • creates mandatory sbt-freestyle files

Note that this is not the final version of docs, since a migration from turtles to droste is undergoing, and that will potentially affect external APIs.

@pepegar pepegar requested a review from a team July 31, 2018 08:49
build.sbt Outdated
@@ -115,11 +115,11 @@ lazy val releaseSettings = {
releasePublishArtifactsAction := PgpKeys.publishSigned.value,
scmInfo := Some(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you might not need this setting, it's being set up by sbt-freestyle (sbt-org-policies).

build.sbt Outdated
)
),
homepage := Some(url("https://github.com/pepegar/skeuomorph")),
homepage := Some(url("https://github.com/frees-io/skeuomorph")),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

build.sbt Outdated
@@ -85,7 +86,6 @@ lazy val releaseSettings = {
setReleaseVersion,
commitReleaseVersion,
tagRelease,
// For non cross-build projects, use releaseStepCommand("publishSigned")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

releaseSettings are also being set up by sbt-freestyle (sbt-org-policies).

@@ -32,44 +51,26 @@ onLoad in Global := { s =>

// General Settings
lazy val commonSettings = Seq(
organization := "com.pepegar",
organization := "io.frees",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add startYear := Some(2018)

namespace <- nonEmptyString
fieldNames <- Gen.nonEmptyContainerOf[Set, String](nonEmptyString).map(_.toList)
fields <- fieldNames.traverse(field)
} yield AvroSchema.createRecord(name, doc, namespace, false, fields.asJava)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use applicative and mapN here to generate all the class fields values in parallel.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'd need this https://github.com/ChristopherDavenport/cats-scalacheck or create an instance of Applicative of Gen

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yap, I already have it in the project

README.md Outdated
.transCata[Mu[freestyle.Schema]](freestyle.utils.transformAvro) // skeuomorph.avro.Schema => skeuomorph.freestyle.Schema
.cata(freestyle.utils.render) // skeuomorph.freestyle.Schema => String
val definition = """
{"namespace": "example.avro",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we improve the formatting? I'd use some online json formatter. For the documentation is better.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure!

README.md Outdated

schema.ana[Mu[avro.Schema]](avro.util.fromAvro). // org.apache.avro.Schema => skeuomorph.avro.Schema
transCata[Mu[freestyle.Schema]](freestyle.util.transformAvro). // skeuomorph.avro.Schema => skeuomorph.freestyle.Schema
cata(freestyle.util.render) // skeuomorph.freestyle.Schema => String
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, what about something like:

schema
  .ana[Mu[avro.Schema]](avro.util.fromAvro) // org.apache.avro.Schema => skeuomorph.avro.Schema
  .transCata[Mu[freestyle.Schema]](freestyle.util.transformAvro) // skeuomorph.avro.Schema =>  skeuomorph.freestyle.Schema
  .cata(freestyle.util.render) // skeuomorph.freestyle.Schema => String

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to format it this way too, but Tut complains about it, probably because it reads the block line by line. That's why I added dots at the end, to tell tut to read next line as part of the expression too.

namespace <- nonEmptyString
fieldNames <- Gen.nonEmptyContainerOf[Set, String](nonEmptyString).map(_.toList)
fields <- fieldNames.traverse(field)
} yield AvroSchema.createRecord(name, doc, namespace, false, fields.asJava)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'd need this https://github.com/ChristopherDavenport/cats-scalacheck or create an instance of Applicative of Gen

build.sbt Outdated
</developers>
}
%%("cats-core"),
%%("specs2-core"),
Copy link

@diesalbla diesalbla Jul 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this line need % Test?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, good cathc!

def createGen(tpe: AvroType): Gen[AvroSchema] =
Gen.const(AvroSchema.create(tpe))

val primitives: Gen[AvroSchema] = Gen.oneOf(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use Gen.oneOf on a Seq[A], without using Gen.const. Also, you can use .map(AvroSchema.create).

createGen(AvroType.NULL)
)

val nonEmptyString: Gen[String] = Gen.nonEmptyListOf(Gen.alphaChar).map(_.mkString)
Copy link

@diesalbla diesalbla Jul 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps Gen.alphaStr.filterNot(_.isEmpty)?

Code

@diesalbla
Copy link

diesalbla commented Jul 31, 2018

@pepegar There are some elements I would add to the README:

  • First, at the beginning I would add a clear phrase stating what skeuomorph is. Some examples from cats and shapeless.

Cats is a library which provides abstractions for functional programming in the Scala programming language. The name is a playful shortening of the word category.

shapeless is a type class and dependent type based generic programming library for Scala. It had its origins in several talks by Miles Sabin (@milessabin), given over the course of 2011, on implementing scrap your boilerplate and higher rank polymorphism in Scala.

  • In particular, whether it is a schema to schema transformation (like the avrohugger), or a coding-encoding library (like avro4s).

  • Another thing I would clarify is what is the relation between this library and freestyle. Does its functionality depend, or is primarilly intended for, either freestyle-core or freestyle-rpc?

  • As a matter of opinion, I would mention the key libraries it is based on. Would those be cats, shapeless, and turtles?

@pepegar pepegar merged commit f3add8a into master Jul 31, 2018
@pepegar pepegar deleted the docs branch July 31, 2018 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants