tuxji commented on code in PR #1236:
URL: https://github.com/apache/daffodil/pull/1236#discussion_r1595486504
##########
.github/workflows/main.yml:
##########
@@ -39,7 +39,7 @@ jobs:
fail-fast: false
matrix:
java_distribution: [ temurin ]
- java_version: [ 8, 11, 17 ]
+ java_version: [ 8, 11, 17, 21 ]
Review Comment:
CI takes between 20 to 30 minutes to complete even before adding Java 21 as
another platform to test. We may want to consider dropping one or two older
platforms. I suggest we keep 8 (I assume we still need to verify compatibility
with legacy systems using Java 8) but drop both 11 and 17. It seems to me,
from an API point of view, that keeping only the earliest and the latest
platforms is enough to cover all the API changes since the API changes are
cumulative from 8 to 21.
If there are any concerns, we can discuss dropping 11 & 17 on the dev list
first and do it in a separate PR instead of doing it in this PR (or hold this
PR until we reach consensus on the dev list).
##########
daffodil-lib/src/main/scala/org/apache/daffodil/lib/util/Misc.scala:
##########
@@ -547,8 +547,8 @@ object Misc {
.onMalformedInput(CodingErrorAction.REPORT)
val bb = ByteBuffer.wrap((0 to 255).map { i => i.toByte }.toArray)
val cb = dec.decode(bb)
- assert(cb.position == 0)
- assert(cb.limit == 256)
+ assert(cb.position() == 0)
+ assert(cb.limit() == 256)
Review Comment:
Oh, good, wish all changes were that easy :-).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
