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

Move scala-xml related code to separate module #610

Merged
merged 5 commits into from
Feb 5, 2019

Conversation

howyp
Copy link
Contributor

@howyp howyp commented Feb 1, 2019

I'm not sure what the naming of this module should be - scala-xml, scalaXml or scalaxml? I tried including the hyphenated version but that seemed to cause problems in the build. Feel free to suggest an alternative to the naming and/or the package layout in the new module!

Note that the core module still depends on scala-xml because the compiler does - see https://github.com/scala/scala-xml/wiki/FAQ

@codecov
Copy link

codecov bot commented Feb 1, 2019

Codecov Report

Merging #610 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #610      +/-   ##
==========================================
- Coverage   93.15%   93.14%   -0.01%     
==========================================
  Files          65       67       +2     
  Lines         716      715       -1     
  Branches       10       10              
==========================================
- Hits          667      666       -1     
  Misses         49       49
Impacted Files Coverage Δ
...red/src/main/scala/eu/timepit/refined/string.scala 100% <ø> (ø) ⬆️
...rc/main/scala/eu/timepit/refined/util/string.scala 100% <ø> (ø) ⬆️
...n/scala/eu/timepit/refined/predicates/string.scala 100% <ø> (ø) ⬆️
.../main/scala/eu/timepit/refined/scalaxml/util.scala 100% <100%> (ø)
...ain/scala/eu/timepit/refined/scalaxml/string.scala 100% <100%> (ø)

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 09bfce8...d1eb212. Read the comment docs.

lazy val scalaxml = myCrossProject("scalaxml")
.dependsOn(core % "compile->compile;test->test")
.settings(
libraryDependencies += "org.scala-lang.modules" %% "scala-xml" % scalaXmlVersion
Copy link
Owner

Choose a reason for hiding this comment

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

If scala-xml is available for Scala.js and Scala Native, shouldn't this use %%%?

Copy link
Owner

@fthomas fthomas left a comment

Choose a reason for hiding this comment

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

scalaxml as module name is fine with me. The module directory should then be lowercase scalaxml too.

@howyp
Copy link
Contributor Author

howyp commented Feb 3, 2019

Sadly, it looks like there's no scala-xml for native:

sbt.librarymanagement.ResolveException: unresolved dependency: org.scala-lang.modules#scala-xml_native0.3_2.11;1.1.1: not found

@howyp
Copy link
Contributor Author

howyp commented Feb 4, 2019

Ok, looks like this is GTG. Merge at will!

@fthomas fthomas changed the base branch from master to series/0.10 February 5, 2019 19:03
Copy link
Owner

@fthomas fthomas left a comment

Choose a reason for hiding this comment

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

Thanks Howy!

I'm going to merge this into the series/0.10 branch so that I can still make 0.9.x releases from master.

@fthomas fthomas merged commit 5aae866 into fthomas:series/0.10 Feb 5, 2019
@fthomas fthomas mentioned this pull request Jan 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants