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

Scala 2.13.0-M5 support #2393

Merged
merged 52 commits into from Apr 2, 2019

Conversation

Projects
None yet
4 participants
@aeons
Copy link
Member

aeons commented Feb 7, 2019

So far only dependencies have been updated.

Closes #2025

aeons and others added some commits Feb 6, 2019

Update build.sbt
Using a locally published parboiled2
@SethTisue

This comment has been minimized.

Copy link
Contributor

SethTisue commented Feb 20, 2019

I don't see a 2.13 job in the CI matrix?

@aeons

This comment has been minimized.

Copy link
Member Author

aeons commented Feb 20, 2019

This is far from building at all. There are several classes that implement collection-like behaviour, and I'm not 100% sure on how to convert those.

Do we add a separate source tree for 2.13, stop extending collections, or something completely different?

@SethTisue

This comment has been minimized.

Copy link
Contributor

SethTisue commented Feb 20, 2019

Do we add a separate source tree for 2.13, stop extending collections, or something completely different?

I haven't looked at http4s's custom collections in particular, but I can offer some general advice in this area.

Very likely you will need some Scala-version-specific sources. (sbt makes this surprisingly easy to arrange.)

It is not usually necessary to copy-and-paste sources wholesale; it's usually possible to get by with just a few aliases and forwarders in version-specific source files.

an example: https://github.com/scala/scala-xml/blob/master/shared/src/main/scala-2.13/scala/xml/ScalaVersionSpecific.scala and https://github.com/scala/scala-xml/blob/master/shared/src/main/scala-2.11-2.12/scala/xml/ScalaVersionSpecific.scala , which are wired in to the build here and used here

another approach is to take on scala-collection-compat as a dependency. this can require less work on your end, but has all the downsides that adding dependencies always has

@aeons

This comment has been minimized.

Copy link
Member Author

aeons commented Feb 22, 2019

Alright. Thanks for the pointers. I'll take another look, when I have some free time.

@rossabaker

This comment has been minimized.

Copy link
Member

rossabaker commented Feb 22, 2019

I haven't looked, but someone said fs2 has scala-collection-compat as a dependency already, so we may already have it.

@rossabaker

This comment has been minimized.

Copy link
Member

rossabaker commented Mar 29, 2019

@SethTisue I'll open a stub ticket now and try to minimize it after the conference.

@ChristopherDavenport

This comment has been minimized.

Copy link
Member

ChristopherDavenport commented Mar 29, 2019

It lives!

@ChristopherDavenport
Copy link
Member

ChristopherDavenport left a comment

👍 to Ross's work. Need some eyes on the build stages / release step changes.

releaseStepCommandAndRemaining("+publishSigned").when(publishable),
releaseStepCommand("sonatypeReleaseAll").when(publishable && release),
releaseStepCommand("docs/ghpagesPushSite").when(publishable && primary),
releaseStepCommand("website/ghpagesPushSite").when(publishable && primary && master),
// releaseStepCommand("docs/ghpagesPushSite").when(publishable && primary),

This comment has been minimized.

Copy link
@ChristopherDavenport

ChristopherDavenport Mar 29, 2019

Member

Moved to docs release stage

releaseStepCommand("test:scalafmt::test").when(primary),
releaseStepCommand("docs/makeSite").when(primary),
releaseStepCommand("website/makeSite").when(primary),
// runTest,

This comment has been minimized.

Copy link
@ChristopherDavenport

ChristopherDavenport Mar 29, 2019

Member

Should we add this and then suppress in the ci step?

releaseStepCommand("website/makeSite").when(primary),
// runTest,
releaseStepCommandAndRemaining("+mimaReportBinaryIssues"),
// releaseStepCommand("unusedCompileDependenciesTest"),

This comment has been minimized.

Copy link
@ChristopherDavenport

ChristopherDavenport Mar 29, 2019

Member

Checked in initial build

// runTest,
releaseStepCommandAndRemaining("+mimaReportBinaryIssues"),
// releaseStepCommand("unusedCompileDependenciesTest"),
// releaseStepCommand("test:scalafmt::test").when(primary),

This comment has been minimized.

Copy link
@ChristopherDavenport

ChristopherDavenport Mar 29, 2019

Member

Checked in primary test build

releaseStepCommandAndRemaining("+mimaReportBinaryIssues"),
// releaseStepCommand("unusedCompileDependenciesTest"),
// releaseStepCommand("test:scalafmt::test").when(primary),
// releaseStepCommand("docs/makeSite").when(primary),

This comment has been minimized.

Copy link
@ChristopherDavenport

ChristopherDavenport Mar 29, 2019

Member

Checked in docs test build

ChristopherDavenport added some commits Mar 29, 2019

@rossabaker

This comment has been minimized.

Copy link
Member

rossabaker commented Mar 30, 2019

I am desperately behind on my talk, so may be releasing this from the plane, but many thanks for jumping in. I'll review the parts I didn't do ASAP.

@aeons

This comment has been minimized.

Copy link
Member Author

aeons commented Apr 1, 2019

Great work, guys!

with Renderable {
override def apply(idx: Int): KeyValue = pairs(idx)
final class Query private (val pairs: Vector[KeyValue]) extends QueryOps with Renderable {
// override def apply(idx: Int): KeyValue = pairs(idx)

This comment has been minimized.

Copy link
@aeons

aeons Apr 1, 2019

Author Member

Does this need a @deprecated or should it be removed?

This comment has been minimized.

Copy link
@rossabaker

rossabaker Apr 2, 2019

Member

I see no reason we can't keep this, though it suggests random access in the implementation. I'm ambivalent about deprecating it or not.

@aeons

This comment has been minimized.

Copy link
Member Author

aeons commented Apr 1, 2019

Apparently I can't approve my own pull request, but I want to do so anyway :)

👍

@rossabaker rossabaker merged commit 67c8b97 into http4s:master Apr 2, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@@ -27,7 +28,7 @@ trait QueryOps {
_withQueryParam(QueryParam[T].key, QueryParamEncoder[T].encode(value) :: Nil)

/** alias for withQueryParam */
def +*?[T: QueryParam: QueryParamEncoder](values: Seq[T]): Self =
def +*?[T: QueryParam: QueryParamEncoder](values: List[T]): Self =

This comment has been minimized.

Copy link
@rossabaker

rossabaker Apr 2, 2019

Member

This is inconsistent with the rest, which take collection.Seq

@aeons aeons deleted the aeons:2.13.0-M5 branch Apr 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.