Skip to content

Refactor BinaryPrimitiveReader and FlexInt - #1129

Merged
popematt merged 2 commits into
amazon-ion:ion11from
popematt:refactorFlexInt
Oct 17, 2025
Merged

popematt merged 2 commits into
amazon-ion:ion11from
popematt:refactorFlexInt

Conversation

@popematt

Copy link
Copy Markdown
Contributor

Issue #, if available:

None

Description of changes:

Cleans up BinaryPrimitiveReader and FlexInt so that they don't have competing concerns.

  • moved/renamed bytecode.BinaryPrimitiveReader to bytecode.bin11.bytearray.PrimitiveDecoder
  • renamed FlexInt to PrimitiveEncoder
  • Moved all read functions from FlexInt to the new PrimitiveDecoder
  • Updated tests, and created PrimitiveTestCases_1_1 that has shared method sources of test cases for reading/writing FlexInts and FlexUInts.

Unfortunately, the diff on github makes it look like FlexInt was renamed to PrimitiveDecoder because of all of the code that was moved.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@popematt
popematt requested a review from austnwil October 17, 2025 16:49
@codecov

codecov Bot commented Oct 17, 2025 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.02985% with 8 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (ion11@beeecb3). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...n/ion/bytecode/bin11/bytearray/PrimitiveDecoder.kt 81.81% 0 Missing and 6 partials ⚠️
...n/java/com/amazon/ion/impl/bin/PrimitiveEncoder.kt 98.01% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             ion11    #1129   +/-   ##
========================================
  Coverage         ?   68.72%           
  Complexity       ?     6051           
========================================
  Files            ?      194           
  Lines            ?    24769           
  Branches         ?     4340           
========================================
  Hits             ?    17023           
  Misses           ?     6423           
  Partials         ?     1323           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@austnwil austnwil left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall. One consistency thing: the Fixed(U)Int reader functions are modeled as extensions on ByteArray, while the Flex(U)Int reader functions accept the ByteArray as a parameter. Might be worth picking one or the other

Comment thread src/test/java/com/amazon/ion/bytecode/bin11/bytearray/PrimitiveDecoderTest.kt Outdated
@popematt
popematt merged commit 10d510d into amazon-ion:ion11 Oct 17, 2025
30 of 36 checks passed
@popematt

Copy link
Copy Markdown
Contributor Author

I will deal with the naming consistency in my next PR.

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