Skip to content

Remove dependency on scala-xml #5597

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

Closed
allanrenucci opened this issue Dec 11, 2018 · 4 comments
Closed

Remove dependency on scala-xml #5597

allanrenucci opened this issue Dec 11, 2018 · 4 comments

Comments

@allanrenucci
Copy link
Contributor

We still bring in scala-xml to run our tests. We should:

  • Report an error when compiling xml literals and scala-xml is not on the classpath similarly to scalac.
  • -Ystop-after:parser for tests that parse xml
  • And then completely remove the dependency
@smarter
Copy link
Member

smarter commented Dec 12, 2018

-Ystop-after:parser for tests that parse xml

but there could be issues related to xml support that only manifest themselves after parsing (e.g. I think #3235 fixed a crash in Typer#assertPositioned)

@allanrenucci
Copy link
Contributor Author

but there could be issues related to xml support that only manifest themselves after parsing

True. But is seems to be the scalac approach (to be confirmed) so I wouldn't try harder than scalac to have good test coverage for xml literals

e.g. I think #3235 fixed a crash in Typer#assertPositioned

I can only find calls to checkPos from: https://github.com/lampepfl/dotty/blob/7c8dee571d936433925befc56edffd00834654dc/compiler/src/dotty/tools/dotc/typer/FrontEnd.scala#L49

Nothing prevent us to keep checking this after parsing

@smarter
Copy link
Member

smarter commented Dec 13, 2018

so I wouldn't try harder than scalac

scalac has the advantage that scala-xml is built using it and has its own test suite, if we add scala-xml to our community build and run its tests then I guess that would be good enough too.

@SethTisue
Copy link
Member

scala-xml would probably accept a pull request adding Dotty to their .travis.yml. (IMO this would complement, not replace, adding scala-xml to the Dotty community build.)

smarter added a commit to dotty-staging/dotty that referenced this issue Jun 2, 2019
…build

This means that our test suites can no longer contain xml tests, instead
they need to be added to scala-xml like this:
scala/scala-xml@9762438
smarter added a commit to dotty-staging/dotty that referenced this issue Jun 2, 2019
…build

This means that our test suites can no longer contain xml tests, instead
they need to be added to scala-xml like this:
scala/scala-xml@9762438
smarter added a commit to dotty-staging/dotty that referenced this issue Jun 2, 2019
…build

This means that our test suites can no longer contain xml tests, instead
they need to be added to scala-xml like this:
scala/scala-xml@9762438
smarter added a commit to dotty-staging/dotty that referenced this issue Jun 2, 2019
…build

This means that our test suites can no longer contain xml tests, instead
they need to be added to scala-xml like this:
scala/scala-xml@9762438
smarter added a commit to dotty-staging/dotty that referenced this issue Jun 2, 2019
…build

This means that our test suites can no longer contain xml tests, instead
they need to be added to scala-xml like this:
scala/scala-xml@9762438
smarter added a commit to dotty-staging/dotty that referenced this issue Jun 3, 2019
…build

This means that our test suites can no longer contain xml tests, instead
they need to be added to scala-xml like this:
scala/scala-xml@9762438
smarter added a commit to dotty-staging/dotty that referenced this issue Jun 3, 2019
…build

This means that our test suites can no longer contain xml tests, instead
they need to be added to scala-xml like this:
scala/scala-xml@9762438
smarter added a commit to dotty-staging/dotty that referenced this issue Jun 3, 2019
…build

This means that our test suites can no longer contain xml tests, instead
they need to be added to scala-xml like this:
scala/scala-xml@9762438
liufengyun added a commit that referenced this issue Jun 3, 2019
Fix #5597: Remove scala-xml dependency, add it to the community-build
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants