Skip to content

Use native json lib#131

Merged
mikpe merged 7 commits into
klarna:masterfrom
zmstone:use-native-json-lib
Nov 24, 2025
Merged

Use native json lib#131
mikpe merged 7 commits into
klarna:masterfrom
zmstone:use-native-json-lib

Conversation

@zmstone

@zmstone zmstone commented Nov 2, 2025

Copy link
Copy Markdown
Contributor

fixes: #128

@mikpe

mikpe commented Nov 4, 2025

Copy link
Copy Markdown
Member

I'll give this a spin in our internal CI sometime this week. @zmstone

Comment thread test/avro_tests.erl Outdated
assert_struct_equal(avro_json, Expect, Got) ->
assert_strcut_equal_no_fields_order(Expect, Got).

assert_strcut_equal_no_fields_order(Value, Value) ->

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: s/strcut/struct/g

@mikpe

mikpe commented Nov 5, 2025

Copy link
Copy Markdown
Member

Locally we add xref checks and they fail with OTP-26. I'll have to figure out the best solution for those.

Comment thread src/avro_json_compat.erl
json ->
iolist_to_binary(json:encode(Value, fun encoder/2));
Module ->
apply(Module, encode, [Value, Options])

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: why the apply instead of simply Module:encode(Value, Options)? (likewise below for decode)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it's due to elvis style check.

# src/avro_json_compat.erl [FAIL]
  - invalid_dynamic_call (https://github.com/inaka/elvis_core/tree/main/doc_rules/elvis_style/invalid_dynamic_call.md)
    - Remove the dynamic function call on line 95. Only modules that define callbacks should make dynamic calls.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That is one stupid rule.

Comment thread src/avro_json_compat.erl
@mikpe

mikpe commented Nov 5, 2025

Copy link
Copy Markdown
Member

Locally we add xref checks and they fail with OTP-26. I'll have to figure out the best solution for those.

An xref_ignores in rebar.config silences those, so I'll go with that locally.

@mikpe mikpe closed this Nov 5, 2025
@mikpe mikpe reopened this Nov 5, 2025
@mikpe

mikpe commented Nov 5, 2025

Copy link
Copy Markdown
Member

Sorry, GH tricked me there.

@mikpe

mikpe commented Nov 5, 2025

Copy link
Copy Markdown
Member

I'm getting dialyzer errors in application code using erlavro, in particular in that "export" application you worked on some years ago. I'll keep looking.

@mikpe

mikpe commented Nov 5, 2025

Copy link
Copy Markdown
Member

Specifically, avro_primitive:type(string, [{<<"orig_type">>, OrigType}]) now fails dialyzer when OrigType is an atom() like integer etc. @zmstone

@zmstone

zmstone commented Nov 17, 2025

Copy link
Copy Markdown
Contributor Author

thanks for the review @mikpe , pushed a few fixes.

@mikpe mikpe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Passed internal testing.

@mikpe
mikpe merged commit f7dbd29 into klarna:master Nov 24, 2025
3 checks passed
@mikpe

mikpe commented Nov 24, 2025

Copy link
Copy Markdown
Member

Tagged 2.11.0 and published to hex. Thanks @zmstone

@zmstone
zmstone deleted the use-native-json-lib branch November 24, 2025 14:02
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.

Support for other JSON libraries

2 participants