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.

Sign up for GitHub

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

Jump to bottom

feat: support init properties #52

Merged
merged 1 commit into from Mar 15, 2022
Merged

feat: support init properties #52

merged 1 commit into from Mar 15, 2022

Conversation

latonz
Copy link
Contributor

@latonz latonz commented Mar 14, 2022

No description provided.

@latonz latonz self-assigned this Mar 14, 2022
@latonz latonz linked an issue Mar 14, 2022 that may be closed by this pull request
Handle init #11
Closed
@latonz latonz marked this pull request as ready for review March 14, 2022 10:53
@latonz latonz requested a review from CommonGuy March 14, 2022 10:54
@latonz latonz force-pushed the feat/support-init-properties branch from 3ac267f to 2afc80b Compare March 14, 2022 16:20
@codecov-commenter
Copy link

Codecov Report

Merging #52 (2afc80b) into main (077fa72) will decrease coverage by 0.50%.
The diff coverage is 87.58%.

@@            Coverage Diff             @@
##             main      #52      +/-   ##
==========================================
- Coverage   89.83%   89.33%   -0.51%     
==========================================
  Files          66       72       +6     
  Lines        1968     2325     +357     
  Branches      261      299      +38     
==========================================
+ Hits         1768     2077     +309     
- Misses        138      182      +44     
- Partials       62       66       +4
Impacted Files Coverage Δ
...gs/PropertyMappings/ConstructorParameterMapping.cs 57.50% <57.50%> (ø)
...ings/PropertyMappings/PropertyAssignmentMapping.cs 62.79% <62.79%> (ø)
...s/Mappings/PropertyMappings/NullPropertyMapping.cs 66.66% <66.66%> (ø)
...yMappings/PropertyNullDelegateAssignmentMapping.cs 70.58% <75.00%> (ø)
...Builder/NewInstanceObjectPropertyMappingBuilder.cs 85.09% <85.09%> (ø)
src/Riok.Mapperly/Descriptors/DescriptorBuilder.cs 97.91% <100.00%> (+0.02%) ⬆️
...scriptors/MappingBuilder/NullableMappingBuilder.cs 95.83% <100.00%> (-1.54%) ⬇️
...ors/MappingBuilder/ObjectPropertyMappingBuilder.cs 100.00% <100.00%> (+0.53%) ⬆️
...pingBuilder/ObjectPropertyMappingBuilderContext.cs 100.00% <100.00%> (ø)
...Riok.Mapperly/Descriptors/MappingBuilderContext.cs 97.50% <100.00%> (+1.50%) ⬆️
... and 15 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

CommonGuy
CommonGuy reviewed Mar 15, 2022
View reviewed changes
src/Riok.Mapperly/Diagnostics/DiagnosticDescriptors.cs Outdated
public static readonly DiagnosticDescriptor CanNotMapToInitOnlyPropertyPath = new(
"RMG015",
"Can not map to init only property path",
"Can not map from property {0}.{1} of type {2} to init only property path {3}.{4} of type {5}",
Copy link
Contributor

Choose a reason for hiding this comment

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

The correct spelling is "Cannot" as a whole word. Applies to all occurences in this class. Also change the variable name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

test/Riok.Mapperly.IntegrationTests/_snapshots/MapperTest.SnapshotGeneratedSource.verified.cs Outdated
@@ -183,7 +184,8 @@ private Riok.Mapperly.IntegrationTests.Models.InheritanceSubObject MapToInherita

private partial Riok.Mapperly.IntegrationTests.Dto.TestObjectDto MapToDtoInternal(Riok.Mapperly.IntegrationTests.Models.TestObject testObject)
{
var target = new Riok.Mapperly.IntegrationTests.Dto.TestObjectDto(DirectInt(testObject.CtorValue), ctorValue2: DirectInt(testObject.CtorValue2));
var target = new Riok.Mapperly.IntegrationTests.Dto.TestObjectDto(DirectInt(testObject.CtorValue), ctorValue2: DirectInt(testObject.CtorValue2))
{IntInitOnlyValue = DirectInt(testObject.IntInitOnlyValue), };
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have any control over the formatting? Looks kinda strange to me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently we use the high level roslyn syntax apis with NormalizeWhitespace() which results in this formatting. However, I removed the trailing comma which improves it a little. If we wanted to control this, we would ned to remove the normalize whitespace call and add whitespaces manually everywhere which wouldn't be effective IMO.

@latonz latonz force-pushed the feat/support-init-properties branch 3 times, most recently from bb3e6a3 to bd0b914 Compare March 15, 2022 07:38
@latonz latonz requested a review from CommonGuy March 15, 2022 07:38
@latonz latonz force-pushed the feat/support-init-properties branch from bd0b914 to 8b61c73 Compare March 15, 2022 07:40
@CommonGuy CommonGuy merged commit 7311cd8 into main Mar 15, 2022
@CommonGuy CommonGuy deleted the feat/support-init-properties branch March 15, 2022 07:46
@github-actions
Copy link

🎉 This PR is included in version 2.2.0-next.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions
Copy link

🎉 This PR is included in version 2.2.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
Reviewers

@CommonGuy CommonGuy CommonGuy approved these changes

Assignees

@latonz latonz

Labels
released on @next released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Handle init
3 participants
@latonz @codecov-commenter @CommonGuy