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

Add support for Play 2.6 #1992

Closed
wants to merge 1 commit into from
Closed

Conversation

benmccann
Copy link
Contributor

@benmccann benmccann commented May 10, 2017

Context

Adds support for Play 2.6, which will soon be released. I'm a committer to Play and employee of LinkedIn

Contributor Checklist

  • Review Contribution Guidelines
  • Sign Gradle CLA
  • [N/A] Link to Design Spec for changes that affect more than 1 public API (that is, not in an internal package) or updates to > 20 files
  • Provide integration tests (under <subproject>/src/integTest) to verify changes from a user perspective
  • Provide unit tests (under <subproject>/src/test) to verify logic
  • Update User Guide, DSL Reference, and Javadoc for public-facing changes
  • Ensure that tests pass locally: ./gradlew quickCheck :platformPlay:check

Gradle Core Team Checklist

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation including proper use of @since and @Incubating annotations for all public APIs
  • Recognize contributor in release notes

@big-guy
Copy link
Member

big-guy commented May 19, 2017

Hey @benmccann. Thanks again for the PR. I'll be taking a look at this for the next release too. I'll let you know when I get back around to this.

@wsargent
Copy link

@big-guy @benmccann Play 2.6.0 is generally available as of today, so it'd be good to get this in.

@big-guy
Copy link
Member

big-guy commented Jul 20, 2017

Hey @benmccann @wsargent, I started running this through our CI infrastructure.

Should there be a com.typesafe.play:run-support_2.11:2.6.0 published somewhere? I see one for M1 and M2, but nothing for anything later.
http://search.maven.org/#search%7Cgav%7C1%7Cg%3A%22com.typesafe.play%22%20AND%20a%3A%22run-support_2.11%22

I also checked the typesafe repos, JCenter and run-support_2.12, and didn't see a 2.6.0 version anywhere.

@benmccann
Copy link
Contributor Author

I see there's a 2.10 version published on Maven. I'm guessing we don't publish 2.11 and 2.12 versions because that library is only used by our SBT plugin, which only runs Scala 2.10

@big-guy
Copy link
Member

big-guy commented Jul 20, 2017

Ah, I didn't think about going down a version :)

It looks like the only part we actually use from that library is play.runsupport.AssetsClassLoader, so it may be easy for us to just provide our own copy of that class.

@big-guy big-guy mentioned this pull request Aug 2, 2017
9 tasks
@benmccann
Copy link
Contributor Author

@big-guy we can publish a 2.11 version of the library. Filed an issue in the Play tracker playframework/playframework#7687

@big-guy
Copy link
Member

big-guy commented Aug 2, 2017

I attempted to just remove our dependency on runsupport: d489608

So we don't have to have a 2.11 version of it.

@benmccann
Copy link
Contributor Author

Ok. That looks simpler anyway

case PLAY_2_6_X:
// TODO: create a TwirlJavaCompiler for Twirl 1.3.x that uses the Java compiler interface
// Rename TwirlCompiler to TwirlScalaCompiler
// Waiting for https://github.com/playframework/twirl/pull/136
Copy link
Contributor

Choose a reason for hiding this comment

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

playframework/twirl#136 was already merged and released. Maybe this can be updated to use play.japi.twirl.compiler.TwirlCompiler.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, that was the plan. I need #2062 to be merged first though

@big-guy big-guy added this to the 4.2 RC1 milestone Aug 5, 2017
@big-guy
Copy link
Member

big-guy commented Aug 6, 2017

I merged this into master (with some fixes for our integration tests). It'll be in 4.2

@big-guy big-guy closed this Aug 6, 2017
@benmccann
Copy link
Contributor Author

Awesome. Thanks!

@ov7a ov7a removed this from the 4.2 RC1 milestone Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:feature A new functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants