Skip to content

Migrate exml attributes from proplists to maps#74

Merged
jacekwegr merged 4 commits intomasterfrom
attrs_as_map
Dec 20, 2024
Merged

Migrate exml attributes from proplists to maps#74
jacekwegr merged 4 commits intomasterfrom
attrs_as_map

Conversation

@NelsonVides
Copy link
Copy Markdown
Collaborator

We could be creating a new version that returns the attributes in a map instead of a proplist, and exports getters in line with maps:with/2 and maps:without/2.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Dec 17, 2024

Codecov Report

❌ Patch coverage is 95.12195% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.92%. Comparing base (809867b) to head (7495721).
⚠️ Report is 20 commits behind head on master.

Files with missing lines Patch % Lines
src/exml.erl 88.23% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #74      +/-   ##
==========================================
+ Coverage   73.87%   73.92%   +0.04%     
==========================================
  Files           7        7              
  Lines        1183     1189       +6     
  Branches      169      169              
==========================================
+ Hits          874      879       +5     
- Misses        309      310       +1     

☔ 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.

Copy link
Copy Markdown

@jacekwegr jacekwegr left a comment

Choose a reason for hiding this comment

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

Looks good to me! 👍 I really like the new utility functions added! I just have one minor comment

src/exml.erl Outdated
xml_sort(Elements) when is_list(Elements) ->
lists:sort([ xml_sort(E) || E <- Elements ]).

%% @doc Return the given `t:element/0' without any `t:cdata/0' on its children.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm not sure this comment is entirely correct

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Oh, copy-paste from the function below, fixing in a sec!

@jacekwegr jacekwegr merged commit 2f1b6e8 into master Dec 20, 2024
@jacekwegr jacekwegr deleted the attrs_as_map branch December 20, 2024 14:31
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