deprecate XMLEventReader?

[SethTisue]

it's

1. janky, proof-of-concept code from at least a decade ago
2. probably not much used, I certainly wouldn't advise it for production code
3. unrelated to the rest of the library
4. in scala.* namespace, which gives people understandably higher expectations of standard-ness and quality

maybe we should deprecate it?

if someone were to get interested in it, they could always bring it back to life as a separate library (not under scala.* this time)

[SethTisue]

I made a label https://github.com/scala/scala-xml/labels/XMLEventReader and attached to relevant open issues

[SethTisue]

(note that there are probably other XMLEventReader bugs for which tickets exist in JIRA but no one cared enough to recreate here)

[ashawley]

Sounds good for the reasons you mention. Despite all that, it is popular:

https://github.com/search?l=Scala&q=XMLEventReader&type=Code

[ashawley]

Interestingly, there is an XMLEventReader that started shipping with Java in version 6:

https://docs.oracle.com/javase/6/docs/api/javax/xml/stream/XMLEventReader.html

[SethTisue]

invited comment at https://contributors.scala-lang.org/t/shall-we-deprecate-scala-xml-pull/1636/1

[SethTisue]

Despite all that, it is popular

I'm not sure those search results qualify for being described as "popular". There is some usage out there, clearly.

Interestingly, there is an XMLEventReader that started shipping with Java in version 6

oh, interesting. well, all the more reason to deprecate ours, then.

[mghildiy]

I can take this one.

[SethTisue]

before actually doing anything, let's wait and see what the community response is to the Discourse post, if any

[SethTisue]

resounding silence on Discourse, so I think the next step is to go ahead and deprecate.

and also see if a volunteer wants to move it to a separate repo. @mghildiy you could take either or both tasks if you like

though I wouldn't suggest you do the "move to separate repo" part unless you think it's actually worth your while, I wouldn't suggest you do it only because you're looking to help out

[mghildiy]

As part of message, I should suggest a better alternative to XMLEventReader. Any good candidate for that?

[mghildiy]

..and I guess I don't need to mention "since".

[SethTisue]

I should suggest a better alternative to XMLEventReader

this seems like a safe recommendation:
https://docs.oracle.com/javase/8/docs/api/javax/xml/stream/package-summary.html

I don't know of any Scala-based alternatives.

..and I guess I don't need to mention "since".

not sure what you mean?

[SethTisue]

I don't know of any Scala-based alternatives.

I asked on the Discourse thread, just in case.

[mghildiy]

I meant I can skip Scala version from when this class would be deprecated.

[ashawley]

There are a few things to consider on Seth's proposal for deprecating it:

1. Can it be ripped out and scala-xml can still be built? Presumably, it can. Just need to make sure if there are supporting classes and traits that can and should go to.
2. What should the@deprecated message say?
3. Should we just suggest using javax.xml.stream.XMLEventReader instead?
4. Should it live on as a separate package (in this repo or elsewhere) and that's what the mesage suggests?

[SethTisue]

I meant I can skip Scala version from when this class would be deprecated.

oh, it's not the Scala version unless you're in the scala/scala repo, it's the version of whatever codebase you're in. so in this case it would be "deprecated since 1.1.1", most likely

[mghildiy]

Ok.

[ashawley]

@SethTisue You know anything about pickJarBasedOn? It references XMLEventReader.

[mghildiy]

I would make change for deprecation, and commit it.
And then would experiment in my local repo according to @ashawley's points.

[ashawley]

I'd suggest removing scala.xml.pull.XMLEvent while we're at it. Unfortunately, it was arbitrarily added to SpecialNode and Document. This will probably produce a lot of unnecessary flags to be raised by MiMa compatibility.

[SethTisue]

You know anything about pickJarBasedOn? It references XMLEventReader

not really, but see scala/util/Properties.scala in the scala/scala repo. it looks to me like @adriaanm just randomly picked some class and the choice doesn't matter?

[SethTisue]

I'd suggest removing scala.xml.pull.XMLEvent while we're at it. Unfortunately, it was arbitrarily added to SpecialNode and Document. This will probably produce a lot of unnecessary flags to be raised by MiMa compatibility.

yuck. well we're not removing quite yet, just deprecating. the release that actually removes this stuff should be one where breaking bincompat is okay

[mghildiy]

I removed XMLEventReader from my local repo, and was able to successfully compile and package the project.

[ashawley]

I tried ripping out XMLEventReader in #200. The tests pass, FWIW. Predictably, it also requires a list of MiMa excludes.

[adriaanm]

it looks to me like @adriaanm just randomly picked some class and the choice doesn't matter?

yup :-)

[SethTisue]

someone want to PR the deprecation...?

[ashawley]

I think we can close this one on deprecating XMLEventReader and friends. Should we open up a new ticket for deleting XMLEventReader and friends or wait for after #199 is released?

[SethTisue]

oh right, I forgot that got merged

[SethTisue]

Should we open up a new ticket

well I figure there's no big hurry to actually remove it, the important thing was to deprecate it.

in the scala repo we just figure we'll do a repo-wide sweep for deprecated things at least once a release cycle, but opening a new ticket is also a sensible approach.

[SethTisue]

here is a sample before-and-after for switching user code from scala.xml.pull to javax.xml.stream.events:
https://gist.github.com/SethTisue/2c84c855221bc5a31e129226ade2cb81