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

Upgrade to scala-parser-combinators 2.1.1 #287

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

xerial
Copy link

@xerial xerial commented May 14, 2021

Problem

Use scala-parser-combinator 2.0.0 2.1.1, which is compatible for Scala 2.11 - Scala 3.0.0.

If a project using twitter-util has a dependency to scala-parser-combinator 2.0.0 with sbt 1.5.x, an error like this will be shown:

[info] [error] java.lang.RuntimeException: found version conflict(s) in library dependencies; some are suspected to be binary incompatible:
[info] [error]
[info] [error] 	* org.scala-lang.modules:scala-parser-combinators_2.12:2.0.0 (early-semver) is selected over 1.1.2
[info] [error] 	    +- org.wvlet.airframe:airframe-launcher_2.12:21.4.1-38-0bf13a68-20210513-1748-SNAPSHOT (depends on 2.0.0)
[info] [error] 	    +- org.wvlet.airframe:airframe-control_2.12:21.4.1-38-0bf13a68-20210513-1748-SNAPSHOT (depends on 2.0.0)
[info] [error] 	    +- com.twitter:util-core_2.12:21.4.0                  (depends on 1.1.2)

Related article: https://www.scala-lang.org/blog/2021/02/16/preventing-version-conflicts-with-versionscheme.html

Solution

Upgrade the scala-parser-combinator version of twitter/util or adding this setting to build.sbt:

ThisBuild / libraryDependencySchemes += "org.scala-lang.modules" %% "scala-parser-combinators" % "always"

Result

There should be no behavior change with this upgrade.

@codecov-commenter
Copy link

codecov-commenter commented May 18, 2021

Codecov Report

Merging #287 (6fbb23c) into develop (07d525b) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #287      +/-   ##
===========================================
- Coverage    49.49%   49.49%   -0.01%     
===========================================
  Files          280      280              
  Lines        15809    15809              
  Branches      1005     1051      +46     
===========================================
- Hits          7825     7824       -1     
- Misses        7984     7985       +1     
Impacted Files Coverage Δ
...in/scala/com/twitter/logging/QueueingHandler.scala 93.75% <0.00%> (-6.25%) ⬇️
...core/src/main/scala/com/twitter/util/Promise.scala 78.36% <0.00%> (+0.40%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 07d525b...6fbb23c. Read the comment docs.

@yufangong
Copy link
Contributor

Hi @xerial , thanks for the PR! We usually keep all our repositories at the same version of 3rdparty dependencies, and during the tests, we found some issues in other repositories (Scrooge and Finatra), so this would take longer than expected.

@felixbr
Copy link
Contributor

felixbr commented Jul 29, 2021

Just wanted to mention that this update is also necessary for the Scala3 migration in general (see #290 where I had to include it as well). 🙂

@pjfanning
Copy link
Contributor

@xerial would you consider rebasing this to fix the conflict reported on this PR? There has been some progress on #274

@xerial
Copy link
Author

xerial commented Jun 22, 2022

ok. I'll rebase this PR onto develop

@xerial xerial force-pushed the upgrade-parser-combinator branch from 6fbb23c to 774fd78 Compare June 22, 2022 23:49
@xerial xerial changed the title Upgrade scala-parser-combinators 2.0.0 Upgrade scala-parser-combinators 2.1.1 Jun 22, 2022
@xerial xerial changed the title Upgrade scala-parser-combinators 2.1.1 Upgrade to scala-parser-combinators 2.1.1 Jun 22, 2022
@xerial
Copy link
Author

xerial commented Jul 30, 2022

It's been already rebased. @pjfanning

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants