Feature/attribs on params #74
Merged
+154
−75
Conversation
Added test for annotation capture Updated to lamdafied syntax for SAM type construction Minor changes to permit compilation under JDK 9 Added Kevin Wright as a contributor
|
Not much needs changing, though there are a few things in the comments. Generally looks really good, and lots of nice tweaks elsewhere - I had no issue with any of the material changes! Thank you! |
| @@ -27,13 +27,21 @@ lazy val tests = project | |||
| .settings(moduleName := "magnolia-tests") | |||
| .settings( | |||
| addCompilerPlugin("org.scalamacros" % "paradise" % "2.1.1" cross CrossVersion.full), | |||
| initialCommands in console := """import magnolia.tests._; import magnolia.examples._;""", | |||
propensive
Feb 8, 2018
Owner
Nice. :)
Nice. :)
build.sbt
Outdated
| libraryDependencies ++= Seq( | ||
| "com.fommil" %% "deriving-macro" % "0.9.0", | ||
| "com.fommil" %% "scalaz-deriving" % "0.9.0" | ||
| "com.fommil" %% "scalaz-deriving" % "0.9.0", |
propensive
Feb 8, 2018
Owner
Looks like scalaz-deriving crept back in... can we drop it?
Looks like scalaz-deriving crept back in... can we drop it?
kevinwright
Feb 8, 2018
Author
Contributor
done
done
build.sbt
Outdated
| @@ -84,12 +92,15 @@ lazy val publishSettings = Seq( | |||
| <name>Jon Pretty</name> | |||
| <url>https://github.com/propensive/magnolia/</url> | |||
| </developer> | |||
| <developer> | |||
propensive
Feb 8, 2018
Owner
I've been using the CONTRIBUTORS file to manage this, though I'm not sure what is convention, or best practice, except that using a combination of both almost certainly isn't. Any ideas?
I've been using the CONTRIBUTORS file to manage this, though I'm not sure what is convention, or best practice, except that using a combination of both almost certainly isn't. Any ideas?
kevinwright
Feb 8, 2018
Author
Contributor
done
done
| @@ -25,6 +25,8 @@ trait Subtype[Typeclass[_], Type] { | |||
|
|
|||
| /** partial function defined the subset of values of `Type` which have the type of this subtype */ | |||
| def cast: PartialFunction[Type, SType] | |||
|
|
|||
| override def toString: String = s"Subtype(${typeName.full})" | |||
propensive
Feb 8, 2018
Owner
Very welcome. :)
Very welcome. :)
| @@ -128,7 +142,7 @@ abstract class CaseClass[Typeclass[_], Type] private[magnolia] ( | |||
|
|
|||
| /** constructs a new instance of the case class type | |||
| * | |||
| * Like [[construct]] this method is implemented by Magnolia and let's you construct case class | |||
| * Like [[construct]] this method is implemented by Magnolia and lets you construct case class | |||
propensive
Feb 8, 2018
Owner
Hanging my head in shame.
Hanging my head in shame.
| param.typeclass.show(param.dereference(value)) | ||
| } else { | ||
| val paramStrings = ctx.parameters.map { param => | ||
| val attribStr = if(param.annotations.isEmpty) "" else { |
propensive
Feb 8, 2018
Owner
Nice addition!
Nice addition!
kevinwright
Feb 8, 2018
Author
Contributor
Had to be able to test it somehow :)
Had to be able to test it somehow :)
| implicit val string: Show[String, String] = new Show[String, String] { | ||
| def show(s: String): String = s | ||
| } | ||
| implicit val string: Show[String, String] = (s: String) => s |
propensive
Feb 8, 2018
Owner
identity? ;)
identity? ;)
kevinwright
Feb 8, 2018
Author
Contributor
Nope, tried that...
Nope, tried that...
propensive
Feb 8, 2018
Owner
😢
| @@ -1,5 +1,5 @@ | |||
| addSbtPlugin("com.jsuereth" % "sbt-pgp" % "1.1.0-M1") | |||
| addSbtPlugin("org.xerial.sbt" % "sbt-sonatype" % "2.0") | |||
| addSbtPlugin("com.jsuereth" % "sbt-pgp" % "1.1.0") | |||
propensive
Feb 8, 2018
Owner
Build maintenance: much appreciated!
Build maintenance: much appreciated!
| @@ -1,12 +1,13 @@ | |||
| package magnolia.tests | |||
|
|
|||
| import language.experimental.macros | |||
| import estrapade.{test, TestApp} | |||
| import estrapade.{TestApp, test} | |||
propensive
Feb 8, 2018
Owner
Sure, why not!
Sure, why not!
kevinwright
Feb 8, 2018
Author
Contributor
Blame intellij :)
Blame intellij :)
| }.assert(_ == "scala.Symbol cannot be cast to java.lang.Integer") | ||
| }.assert{x => | ||
| //tiny hack because Java 9 inserts the "java.base/" module name in the error message | ||
| x.startsWith("scala.Symbol cannot be cast to") && x.endsWith("java.lang.Integer") |
propensive
Feb 8, 2018
Owner
That's horrible. But we're in test code now, so pragmatism reigns!
That's horrible. But we're in test code now, so pragmatism reigns!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Added test for annotation capture
Updated to lamdafied syntax for SAM type construction
Minor changes to permit compilation under JDK 9
Added Kevin Wright as a contributor