Skip to content

Conversation

@axunonb
Copy link
Collaborator

@axunonb axunonb commented May 12, 2025

This also simplified the code IMHO.

@codecov
Copy link

codecov bot commented May 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

❌ Your project status has failed because the head coverage (67%) is below the target coverage (80%). You can increase the head coverage or adjust the target coverage.

Impacted file tree graph

@@         Coverage Diff         @@
##           main   #793   +/-   ##
===================================
- Coverage    67%    67%   -0%     
===================================
  Files       106    105    -1     
  Lines      4450   4414   -36     
  Branches   1072   1060   -12     
===================================
- Hits       2965   2936   -29     
+ Misses     1046   1044    -2     
+ Partials    439    434    -5     
Files with missing lines Coverage Δ
Ical.Net/Collections/GroupedList.cs 50% <100%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@axunonb axunonb marked this pull request as ready for review May 12, 2025 12:39
@axunonb axunonb requested a review from minichma May 12, 2025 14:41
@minichma
Copy link
Collaborator

minichma commented May 12, 2025

This also simplified the code IMHO.

Absolutely. But do we need this at all? The only usage I can find is here:

public IEnumerator<TItem> GetEnumerator() => new GroupedListEnumerator<TItem>(_lists);

Couldn't we just implement that like so?

_lists.SelectMany(x => x).GetEnumerator();

@axunonb axunonb force-pushed the wip/axunonb/pr/nrt-grouped-list-enumerator branch from 0fa7818 to 44a3eb1 Compare May 12, 2025 15:48
@sonarqubecloud
Copy link

@axunonb
Copy link
Collaborator Author

axunonb commented May 12, 2025

But do we need this at all

This kind of simplification really goes way to far 😂

@axunonb axunonb changed the title Fix nullability handling in GroupedListEnumerator Remove unnecessary GroupedListEnumerator May 12, 2025
Copy link
Collaborator

@minichma minichma left a comment

Choose a reason for hiding this comment

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

Love it!

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