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

feat: add user implemented void mapping methods #569

Merged
merged 1 commit into from
Aug 7, 2023

Conversation

TimothyMakkison
Copy link
Collaborator

@TimothyMakkison TimothyMakkison commented Jul 16, 2023

Let mapperly use user implemented existing target methods

Description

Add user implemented existing target mapping methods. Mapperly will attempt to use the users methods where possible

I can add support for reusing user defined void mappings, it should be a different PR.

Fixes #439

Checklist

  • The existing code style is followed
  • The commit message follows our guidelines
  • Performed a self-review of my code
  • Hard-to-understand areas of my code are commented
  • The documentation is updated (as applicable)
  • Unit tests are added/updated
  • Integration tests are added/updated (as applicable, especially if feature/bug depends on roslyn or framework version in use)

@TimothyMakkison TimothyMakkison force-pushed the use_existing_meth branch 2 times, most recently from 1d5acfd to ab46a5f Compare July 16, 2023 12:20
@TimothyMakkison TimothyMakkison marked this pull request as draft July 16, 2023 12:27
@TimothyMakkison TimothyMakkison force-pushed the use_existing_meth branch 3 times, most recently from 7b48907 to 0edb879 Compare July 16, 2023 13:30
@codecov
Copy link

codecov bot commented Jul 16, 2023

Codecov Report

Merging #569 (67f3016) into main (3d2ed69) will decrease coverage by 0.07%.
Report is 1 commits behind head on main.
The diff coverage is 86.20%.

❗ Current head 67f3016 differs from pull request most recent head 7efb65d. Consider uploading reports for the commit 7efb65d to get more accurate results

@@            Coverage Diff             @@
##             main     #569      +/-   ##
==========================================
- Coverage   90.53%   90.46%   -0.07%     
==========================================
  Files         173      175       +2     
  Lines        5956     6032      +76     
  Branches      756      762       +6     
==========================================
+ Hits         5392     5457      +65     
- Misses        398      404       +6     
- Partials      166      171       +5     
Files Changed Coverage Δ
...scriptors/InlineExpressionMappingBuilderContext.cs 95.91% <ø> (ø)
...Riok.Mapperly/Descriptors/MappingBuilderContext.cs 100.00% <ø> (ø)
...iptors/MappingBuilders/DictionaryMappingBuilder.cs 90.82% <ø> (ø)
.../MappingBuilders/DirectAssignmentMappingBuilder.cs 100.00% <ø> (ø)
.../Descriptors/MappingBuilders/EnumMappingBuilder.cs 84.75% <ø> (ø)
...tors/MappingBuilders/EnumToStringMappingBuilder.cs 100.00% <ø> (ø)
...iptors/MappingBuilders/EnumerableMappingBuilder.cs 94.83% <ø> (ø)
...escriptors/MappingBuilders/MemoryMappingBuilder.cs 88.46% <ø> (ø)
...uilders/NewInstanceObjectPropertyMappingBuilder.cs 91.17% <ø> (ø)
...criptors/MappingBuilders/NullableMappingBuilder.cs 92.85% <ø> (ø)
... and 46 more

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@TimothyMakkison TimothyMakkison force-pushed the use_existing_meth branch 2 times, most recently from 873b388 to 65551df Compare July 16, 2023 13:54
@TimothyMakkison TimothyMakkison marked this pull request as ready for review July 16, 2023 13:55
@TimothyMakkison TimothyMakkison marked this pull request as draft July 16, 2023 21:06
@TimothyMakkison
Copy link
Collaborator Author

Should I update the docs under user implemented mappings or void mappings?

@latonz
Copy link
Contributor

latonz commented Jul 26, 2023

@TimothyMakkison I'll review the open PR's after 2.9.0 is released (we plan to release it next week). You can probably wait until the code / API changes are reviewed. If sth. fundamental changes after the review on the API surface, you then don't need to rewrite the docs again.

@TimothyMakkison TimothyMakkison force-pushed the use_existing_meth branch 4 times, most recently from 911f838 to 3b6eb39 Compare August 1, 2023 19:16
@TimothyMakkison
Copy link
Collaborator Author

TimothyMakkison commented Aug 1, 2023

  • Made suggested chnages
  • Reworked ITypeMapping

Would it be worth making AddExisting and AddNewInstance private and using MappingCollection.Add directly? Should I create issues for reuse existing target user defined and reuse existing target mapping methods?
I haven't updated the docs.

@latonz
Copy link
Contributor

latonz commented Aug 2, 2023

@TimothyMakkison feel free to create the issues.
I'd keep AddExisting and AddNewInstance public and call them if the type is known. This has a little performance benefit since the runtime type switch is not needed.

@TimothyMakkison TimothyMakkison force-pushed the use_existing_meth branch 2 times, most recently from 67f3016 to c588462 Compare August 2, 2023 09:29
@TimothyMakkison TimothyMakkison marked this pull request as ready for review August 2, 2023 10:51
@TimothyMakkison TimothyMakkison force-pushed the use_existing_meth branch 3 times, most recently from 16e3a6c to 7efb65d Compare August 2, 2023 13:18
@latonz latonz added this to the v3.1.0 milestone Aug 7, 2023
@latonz
Copy link
Contributor

latonz commented Aug 7, 2023

2 Nitpicks and rebasing, afterwards this can get merged 😊

@TimothyMakkison
Copy link
Collaborator Author

2 Nitpicks and rebasing, afterwards this can get merged 😊

Thanks for the review, done the changes. 👍

I think it will be best to update the docs after all the other void mappings are completed

@latonz latonz merged commit ec3416d into riok:main Aug 7, 2023
12 of 14 checks passed
@github-actions
Copy link

github-actions bot commented Aug 8, 2023

🎉 This PR is included in version 3.1.0-next.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions
Copy link

🎉 This PR is included in version 3.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

Pick up user implemented void mapping methods
2 participants