Skip to content

Conversation

roofdiver
Copy link

Hi! This is related to #285. I've added a small example file and details in the unit tests

@roofdiver
Copy link
Author

Hi @martin-g I added the recommended changes. Please let me know if this works.

@roofdiver roofdiver requested a review from martin-g September 18, 2025 18:29
Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
@martin-g martin-g requested a review from Copilot September 20, 2025 21:16
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds unit tests to demonstrate a bug with byte array deserialization in the Apache Avro library, as referenced in issue #285. The changes include comprehensive test cases and documentation to help reproduce and understand the problem.

  • Adds failing unit tests for byte array deserialization scenarios
  • Provides documentation and examples for proper byte array handling with Serde
  • Includes both complete and filtered deserialization test cases to isolate the issue

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
avro/tests/avro-rs-285-bytes_deserialization.rs New test file with comprehensive unit tests demonstrating the byte array deserialization bug
avro/src/lib.rs Documentation section explaining proper Serde usage for byte arrays with detailed examples
avro/README.md User-facing documentation with examples of byte array deserialization patterns

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

martin-g and others added 2 commits September 21, 2025 00:18
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
@roofdiver
Copy link
Author

Hi @martin-g , apologies for the drop off. Is there anything else you need on my end for the doc updates?

@martin-g
Copy link
Member

martin-g commented Oct 2, 2025

Is there anything else you need on my end for the doc updates?

No!

I postponed the merge of few no high prio PRs like this one because of our work on Async APIs (#238).
E.g. #246 makes many changes and it is hard to keep it up to date with the main branch.

This PR will be merged for the next release! With or without the Async APIs.

@martin-g martin-g added this to the 0.21.0 milestone Oct 2, 2025
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.

2 participants