Fix non-prefix agnostic test #1436
Open
+18
−10
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
olabusayoT
requested review from
mbeckerle,
stevedlawrence and
jadams-tresys
olabusayoT
force-pushed
the
daf-2152-fix-non-prefix-agnostic-test
branch
4 times, most recently
from
884a38b to
9e0f821
Compare
- fix test to make prefix agnostic - fix test that was failing because a scala Buffer doesn't automatically convert to a Seq anymore - update nullary method with no side effects to remove paren - fix hard coded scala version in integration tests Deprecation/Compatibility: Note that some changes in 2.13 may affect the order of prefixes with the same namespace and may also cause them to appear uniformly but interchangeably. Ex: `xmlns="urn:bin" xmlns:b="urn:bin" <element/>` may become `xmlns:b="urn:bin" xmlns="urn:bin" <b:element/>` and vice versa. DAFFODIL-2152
olabusayoT
force-pushed
the
daf-2152-fix-non-prefix-agnostic-test
branch
from
9e0f821 to
997a3a1
Compare
jadams-tresys
approved these changes
Feb 14, 2025
Comment on lines
-71
to
+77
| def devNull(): String = if (isWindows) "NUL" else "/dev/null" | ||
| def devNull: String = if (isWindows) "NUL" else "/dev/null" |
There was a problem hiding this comment.
Not a problem but I have to laugh at the irony of removing parens from function definitions and adding them to the function calls.
There was a problem hiding this comment.
Well this one gave me the option to remove the parens from the def or add it to the call so I went with the former :D
mbeckerle
approved these changes
Feb 14, 2025
| @@ -50,7 +50,7 @@ public void testPrimaryConstructor() { | |||
| false, | |||
| NoRoundTrip$.MODULE$, | |||
| "off", | |||
| CollectionConverters.asScala(Arrays.asList("daffodil", "ibm")), | |||
| CollectionConverters.asScala(Arrays.asList("daffodil", "ibm")).toSeq(), | |||
There was a problem hiding this comment.
Why does toSeq need () arguments? It's a pure conversion.
There was a problem hiding this comment.
I think it's because we're in a Java file
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Deprecation/Compatibility:
Note that some changes in 2.13 may affect the order of prefixes with the same namespace and may also cause them to appear uniformly but interchangeably. Ex:
xmlns="urn:bin" xmlns:b="urn:bin" <element/>may becomexmlns:b="urn:bin" xmlns="urn:bin" <b:element/>and vice versa.DAFFODIL-2152