Skip to content

SI-8523 Adjust test for namespace interpolation #36

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
wants to merge 1 commit into from

Conversation

retronym
Copy link
Member

@retronym retronym commented Dec 3, 2014

This test fails after we try to fix SI-8523 in the compiler.
But the test is actually at fault here, as it was relying on
the loose matching performed by the compiler's SymbolXMLBuilder.

The fix for SI-8523 that triggered the failure:

scala/scala@8d175b907

The original commit that introduced this test:

scala/scala@e1ffc05b10be6

Review by @som-snytt

This test fails after we try to fix SI-8523 in the compiler.
But the test is actually at fault here, as it was relying on
the loose matching performed by the compiler's SymbolXMLBuilder.

The fix for SI-8523 that triggered the failure:

   scala/scala@8d175b907

The original commit that introduced this test:

   scala/scala@e1ffc05b10be6
@som-snytt
Copy link
Contributor

LGTM

I'm impressed by the test coverage.

I'm using this pair of PRs to demonstrate that you can fix such bugs in a point release without deprecation.

The original issue was https://issues.scala-lang.org/browse/SI-486 which didn't ask for text nodes.

Personally, I'm not totally clear on the use case for supplying a text node as an attribute value. I just decomposed some xml and I use the text node result (instead of textnode.text) to build some other xml, dynamically setting the ns? Somehow it sounds like cooking a goat in its mother's milk.

@adriaanm
Copy link
Contributor

adriaanm commented Dec 3, 2014

FYI, digits were swapped in the SI-nembur. (It's SI-8253)

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.

3 participants