Skip to content

Commit

Permalink
replace pimp with 'rich', 'enrich', 'extension method'
Browse files Browse the repository at this point in the history
and:
* add MiMa so we know the change is binary compatible
* use current Scala & sbt versions
  • Loading branch information
SethTisue committed Sep 14, 2017
1 parent 8a421f0 commit 7089839
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 18 deletions.
2 changes: 1 addition & 1 deletion .travis.yml
@@ -1,6 +1,6 @@
language: scala
script:
- sbt "+ test"
- sbt "+ test" "+ mimaReportBinaryIssues"
jdk:
- oraclejdk8
notifications:
Expand Down
4 changes: 2 additions & 2 deletions CHANGELOG
@@ -1,4 +1,4 @@
Version 1.3.3 (1016-12-29)
Version 1.3.3 (2016-12-29)
--------------------------

For Scala 2.12, this release brings no updates over 1.3.2 because the 2.12 release
Expand Down Expand Up @@ -102,7 +102,7 @@ Version 1.1.1 (2012-03-13)
Version 1.1.0 (2012-02-01)
--------------------------
- Added automatic case class field name extraction via new jsonFormatX overloads
- Added 'asJson' pimp to Strings
- Added 'asJson' extension method to Strings
- Added RootJsonFormat (JsonFormat for types corresponding to JSON document roots)
- Fixed problem of JSON object deserialization not being member-order independent
(removed JsField, turned JsObject(List[JsField]) into JsObject(Map[String, JsValue]))
Expand Down
2 changes: 1 addition & 1 deletion README.markdown
Expand Up @@ -53,7 +53,7 @@ and do one or more of the following:
val json = jsonAst.prettyPrint // or .compactPrint
```

* Convert any Scala object to a JSON AST using the pimped `toJson` method
* Convert any Scala object to a JSON AST using the `toJson` extension method

```scala
val jsonAst = List(1, 2, 3).toJson
Expand Down
27 changes: 19 additions & 8 deletions build.sbt
@@ -1,6 +1,6 @@
name := "spray-json"

version := "1.3.3"
version := "1.3.4-SNAPSHOT"

organization := "io.spray"

Expand All @@ -14,7 +14,7 @@ startYear := Some(2011)

licenses := Seq("Apache 2" -> new URL("http://www.apache.org/licenses/LICENSE-2.0.txt"))

scalaVersion := "2.11.8"
scalaVersion := "2.11.11"

scalacOptions ++= Seq("-feature", "-language:_", "-unchecked", "-deprecation", "-Xlint", "-encoding", "utf8")

Expand Down Expand Up @@ -44,28 +44,39 @@ osgiSettings

OsgiKeys.exportPackage := Seq("""spray.json.*;version="${Bundle-Version}"""")

OsgiKeys.importPackage <<= scalaVersion { sv => Seq("""scala.*;version="$<range;[==,=+);%s>"""".format(sv)) }
OsgiKeys.importPackage := Seq("""scala.*;version="$<range;[==,=+);%s>"""".format(scalaVersion.value))

OsgiKeys.importPackage ++= Seq("""spray.json;version="${Bundle-Version}"""", "*")

OsgiKeys.additionalHeaders := Map("-removeheaders" -> "Include-Resource,Private-Package")

// Migration Manager
mimaPreviousArtifacts := Set("io.spray" %% "spray-json" % "1.3.3")

///////////////
// publishing
///////////////

crossScalaVersions := Seq("2.10.6", "2.11.8", "2.12.1")
crossScalaVersions := Seq("2.10.6", "2.11.11", "2.12.3")

scalaBinaryVersion <<= scalaVersion(sV => if (CrossVersion.isStable(sV)) CrossVersion.binaryScalaVersion(sV) else sV)
scalaBinaryVersion := {
val sV = scalaVersion.value
if (CrossVersion.isScalaApiCompatible(sV))
CrossVersion.binaryScalaVersion(sV)
else
sV
}

publishMavenStyle := true

useGpg := true

publishTo <<= version { v: String =>
publishTo := {
val nexus = "https://oss.sonatype.org/"
if (v.trim.endsWith("SNAPSHOT")) Some("snapshots" at nexus + "content/repositories/snapshots")
else Some("releases" at nexus + "service/local/staging/deploy/maven2")
if (version.value.trim.endsWith("SNAPSHOT"))
Some("snapshots" at nexus + "content/repositories/snapshots")
else
Some("releases" at nexus + "service/local/staging/deploy/maven2")
}

pomIncludeRepository := { _ => false }
Expand Down
2 changes: 1 addition & 1 deletion notes/1.1.0.markdown
Expand Up @@ -4,7 +4,7 @@ and will have group id `cc.spray` rather than `cc.spray.json` as before.
Changes since the last release (1.0.1):

- Added automatic case class field name extraction via new `jsonFormatX` overloads
- Added `asJson` pimp to Strings
- Added `asJson` extension method to Strings
- Added `RootJsonFormat` (`JsonFormat` for types corresponding to JSON document roots)
- Fixed problem of JSON object deserialization not being member-order independent
(removed `JsField`, turned `JsObject(List[JsField])` into `JsObject(Map[String, JsValue])`)
Expand Down
2 changes: 1 addition & 1 deletion project/build.properties
@@ -1 +1 @@
sbt.version=0.13.9
sbt.version=0.13.16
4 changes: 3 additions & 1 deletion project/plugins.sbt
Expand Up @@ -2,4 +2,6 @@ addSbtPlugin("io.spray" % "sbt-boilerplate" % "0.5.9")

addSbtPlugin("com.typesafe.sbt" % "sbt-osgi" % "0.7.0")

addSbtPlugin("com.jsuereth" % "sbt-pgp" % "1.0.0")
addSbtPlugin("com.jsuereth" % "sbt-pgp" % "1.0.0")

addSbtPlugin("com.typesafe" % "sbt-mima-plugin" % "0.1.18")
24 changes: 21 additions & 3 deletions src/main/scala/spray/json/package.scala
Expand Up @@ -25,23 +25,41 @@ package object json {

def jsonReader[T](implicit reader: JsonReader[T]) = reader
def jsonWriter[T](implicit writer: JsonWriter[T]) = writer

implicit def pimpAny[T](any: T) = new PimpedAny(any)
implicit def pimpString(string: String) = new PimpedString(string)

implicit def enrichAny[T](any: T) = new RichAny(any)
implicit def enrichString(string: String) = new RichString(string)

@deprecated("use enrichAny", "1.3.4")
def pimpAny[T](any: T) = new PimpedAny(any)
@deprecated("use enrichString", "1.3.4")
def pimpString(string: String) = new PimpedString(string)
}

package json {

case class DeserializationException(msg: String, cause: Throwable = null, fieldNames: List[String] = Nil) extends RuntimeException(msg, cause)
class SerializationException(msg: String) extends RuntimeException(msg)

private[json] class RichAny[T](any: T) {
def toJson(implicit writer: JsonWriter[T]): JsValue = writer.write(any)
}

private[json] class RichString(string: String) {
@deprecated("deprecated in favor of parseJson", "1.2.6")
def asJson: JsValue = parseJson
def parseJson: JsValue = JsonParser(string)
}

@deprecated("use RichAny", "1.3.4")
private[json] class PimpedAny[T](any: T) {
def toJson(implicit writer: JsonWriter[T]): JsValue = writer.write(any)
}

@deprecated("use RichString", "1.3.4")
private[json] class PimpedString(string: String) {
@deprecated("deprecated in favor of parseJson", "1.2.6")
def asJson: JsValue = parseJson
def parseJson: JsValue = JsonParser(string)
}

}

3 comments on commit 7089839

@dclements
Copy link

Choose a reason for hiding this comment

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

In the future I would like to ask that breaking changes to the API interface involve more than a bugfix-level point change or at least a migration guide or changelog change to document that the change is breaking.

Specifically, this removal:

-  implicit def pimpAny[T](any: T) = new PimpedAny(any)
-  implicit def pimpString(string: String) = new PimpedString(string)

and replacing it with:

@deprecated("use enrichAny", "1.3.4")
 +  def pimpAny[T](any: T) = new PimpedAny(any)
 +  @deprecated("use enrichString", "1.3.4")
 +  def pimpString(string: String) = new PimpedString(string)

Meant that any code that depended on the implicit broke.

I like the renaming, definitely +1 for that, but the break was a bit jarring, especially with no documentation on what changed/changelog/notification.

@SethTisue
Copy link
Contributor Author

@SethTisue SethTisue commented on 7089839 Nov 7, 2017

Choose a reason for hiding this comment

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

@dclements the deprecated implicits were replaced with new implicits that just have a different name, so we expected that the change would be source compatible for 99% of users, since we thought basically nobody would have cause to refer to the old implicits by name.

and leaving the old methods in place, minus implicit, should have ensured that the change was binary compatible.

Meant that any code that depended on the implicit broke

I don't understand why that would be. What broke for you? I'd like to understand this better. The change was intended to be 100% binary compatible, and source compatible for nearly everybody. Did we screw it up?

@dclements
Copy link

@dclements dclements commented on 7089839 Nov 7, 2017

Choose a reason for hiding this comment

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

I think that there's an implicit (heh) assumption that people using the library will import implicits by _ and not by name, but frequently in order to bound the scope or for style guide reasons just the name is imported (e.g., IntelliJ has a Class count to use import with '_' setting in Editor > Code Style > Scala). This means that a lot of the time when importing implicits the scope is going to be bound by a combination of location (which isn't a problem here except in that it means fewer imports so editors are less likely to use _) and/or specificity (which is why it broke for me) .

To make this concrete about my use case, let's say that I have a JSON string:

val foo = """{}"""

Previously to convert foo to json I would call

import spray.json.pimpString
 
foo.parseJson

Which now breaks.

Please sign in to comment.