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.

Sign up for GitHub

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

Jump to bottom

Build: don't depend on Interplay #468

Merged
merged 7 commits into from
May 26, 2020
Merged

Build: don't depend on Interplay #468

merged 7 commits into from
May 26, 2020

Conversation

ennru
Copy link
Contributor

@ennru ennru commented May 25, 2020

Preparations to enable release by tag

  • Remove Interplay plugin to disentangle builds
  • Copy in parts from Interplay

octonato
octonato reviewed May 25, 2020
View reviewed changes
build.sbt Show resolved Hide resolved
octonato
octonato approved these changes May 25, 2020
View reviewed changes
Copy link
Contributor

@octonato octonato left a comment

Choose a reason for hiding this comment

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

Looking good. I think we can move to next step: tweaking Travis

@ennru ennru requested a review from ignasi35 May 25, 2020 11:05
octonato
octonato reviewed May 25, 2020
View reviewed changes
build.sbt Outdated Show resolved Hide resolved
octonato
octonato reviewed May 25, 2020
View reviewed changes
project/Common.scala Outdated
"https://gitter.im/playframework/contributors",
url("https://github.com/playframework")
),
description := "Play mailer plugin"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
description := "Play mailer plugin"
description := "Play JSON"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😄

ignasi35
ignasi35 suggested changes May 25, 2020
View reviewed changes
Copy link
Member

@ignasi35 ignasi35 left a comment

Choose a reason for hiding this comment

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

It's so good to see interplay go. Thanks @ennru

build.sbt Show resolved Hide resolved
build.sbt
@@ -139,27 +128,13 @@ lazy val root = project
`play-json-joda`
)
.settings(commonSettings)
.settings(
Seq(
// this overrides releaseProcess to make it work with sbt-dynver
Copy link
Member

Choose a reason for hiding this comment

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

OMG! Why use interplay if the release process is overriden?!

So good to get rid of this!

build.sbt Outdated

lazy val `play-json` = crossProject(JVMPlatform, JSPlatform)
.crossType(CrossType.Full)
.in(file("play-json"))
.enablePlugins(PlayLibrary, Playdoc)
// .enablePlugins(PlayLibrary, Playdoc)
.enablePlugins(Publish, Playdoc)
Copy link
Member

Choose a reason for hiding this comment

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

I'm susprised this one has the Playdoc plugin but not Omnidoc one.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, I think it does need Omnidoc. Interplays PlayLibrary` did bring it in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

project/Dependencies.scala Outdated
@@ -0,0 +1,9 @@
object Dependencies {
// scalaVersion needs to be kept in sync with travis-ci
val Scala210 = "2.10.7"
Copy link
Member

Choose a reason for hiding this comment

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

Is this needed for scalaJS?
I think interplay only had scala210 for sbt 0.13...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed.

project/Omnidoc.scala
Comment on lines +44 to +45
case None if buildDir == projDir => "" // Same dir (sbt 0.13)
case Some("") => "" // Same dir (sbt 1.0)
Copy link
Member

Choose a reason for hiding this comment

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

Probably we can get rid of these two cases in play-json

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to keep the diff to Interplay's version minimal.

@ennru ennru force-pushed the remove-interplay branch from a7ccd6c to 730c36d Compare May 25, 2020 17:32
ennru
ennru commented May 25, 2020
View reviewed changes
Copy link
Contributor Author

@ennru ennru left a comment

Choose a reason for hiding this comment

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

And rebased on master.

build.sbt Show resolved Hide resolved
build.sbt Outdated

lazy val `play-json` = crossProject(JVMPlatform, JSPlatform)
.crossType(CrossType.Full)
.in(file("play-json"))
.enablePlugins(PlayLibrary, Playdoc)
// .enablePlugins(PlayLibrary, Playdoc)
.enablePlugins(Publish, Playdoc)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

project/Common.scala Outdated
"https://gitter.im/playframework/contributors",
url("https://github.com/playframework")
),
description := "Play mailer plugin"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

😄

project/Dependencies.scala Outdated
@@ -0,0 +1,9 @@
object Dependencies {
// scalaVersion needs to be kept in sync with travis-ci
val Scala210 = "2.10.7"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed.

project/Omnidoc.scala
Comment on lines +44 to +45
case None if buildDir == projDir => "" // Same dir (sbt 0.13)
case Some("") => "" // Same dir (sbt 1.0)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to keep the diff to Interplay's version minimal.

@ennru ennru marked this pull request as ready for review May 25, 2020 17:51
@octonato
Copy link
Contributor

Is this ready for merge?

@ennru ennru merged commit 9462d1c into playframework:master May 26, 2020
@ennru ennru deleted the remove-interplay branch May 26, 2020 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@octonato octonato octonato approved these changes

@ignasi35 ignasi35 ignasi35 requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ennru @octonato @ignasi35