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

[COR-123] Support records #157

Merged
merged 10 commits into from
Dec 12, 2023
Merged

[COR-123] Support records #157

merged 10 commits into from
Dec 12, 2023

Conversation

wojtek2288
Copy link
Contributor

Assumed, that we do not want clients to have distinction between record type DTOs and class type DTOs.

Copy link
Member

@jakubfijalkowski jakubfijalkowski left a comment

Choose a reason for hiding this comment

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

I would also add a test for a record being command/query/opertation.
Plus, this needs fixing.

@jakubfijalkowski
Copy link
Member

Also, forgot about it - changelog change is needed :)

Copy link

github-actions bot commented Dec 12, 2023

Test Results

    2 files  ±  0      2 suites  ±0   44s ⏱️ -18s
  82 tests +  6    82 ✔️ +  6  0 💤 ±0  0 ±0 
178 runs  +12  178 ✔️ +12  0 💤 ±0  0 ±0 

Results for commit c24a606. ± Comparison against base commit 8628cca.

♻️ This comment has been updated with latest results.

@codecov-commenter
Copy link

codecov-commenter commented Dec 12, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8628cca) 81.76% compared to head (1306349) 81.97%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #157      +/-   ##
==========================================
+ Coverage   81.76%   81.97%   +0.20%     
==========================================
  Files          37       37              
  Lines        1300     1315      +15     
  Branches      127      130       +3     
==========================================
+ Hits         1063     1078      +15     
  Misses        190      190              
  Partials       47       47              
Flag Coverage Δ
net6.0 81.79% <100.00%> (+0.21%) ⬆️
net7.0 81.79% <100.00%> (+0.21%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

public record DTO1(int Property);
public record DTO2(string Property);

public class Command : ICommand
Copy link
Member

Choose a reason for hiding this comment

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

I meant that we should check if this will work:

public record Command(DTO1 DTO1) : ICommand;

et al. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Huh, didn't think about this one, but it looks like it works as well

@wojtek2288 wojtek2288 merged commit 65f572f into main Dec 12, 2023
4 checks passed
@wojtek2288 wojtek2288 deleted the feature/add-records branch December 12, 2023 12:48
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.

4 participants