Skip to content
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

Fix pb options packed regression #247

Merged
merged 2 commits into from
Jun 7, 2024

Conversation

Lupus
Copy link
Contributor

@Lupus Lupus commented Jun 7, 2024

Pb_parsing_util was adding "packed" option for float repeated fields in proto3 syntax unconditionally. In previous implementation there was a single Pb_option module with last-write-wins semantics, after refactoring of options there was Pb_raw_option with compilation phase to Pb_option and different semantics: adding more "packed" field resulted in it being compiled to "packed": [true,true], as repetion of options in Protobuf means it's a destructured list. That unconditional overriding of option stopped working as expected, and as integration tests were not run during dune runtest, this went under the radars unfortunately when I was introducing options refactoring. I've added an explicit test which catches this problem, and separate commit which fixes this behavior (via new Pb_raw_option.add_or_replace which removes previous values if any).

@c-cube please take a look. I've verified that this fixes the test run and build on your branch with resurrection of integration tests.

@c-cube c-cube merged commit 13f9559 into mransan:br-4.0 Jun 7, 2024
3 checks passed
@c-cube
Copy link
Collaborator

c-cube commented Jun 7, 2024

Thank you!

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