Skip to content

Conversation

@axunonb
Copy link
Collaborator

@axunonb axunonb commented Apr 7, 2025

Methods left using DateTime:

Type: Ical.Net.Calendar, Method: AddTimeZone
Type: Ical.Net.Calendar, Method: AddTimeZone
Type: Ical.Net.Calendar, Method: AddLocalTimeZone
Type: Ical.Net.Utility.DateUtil, Method: AsCalDateTime
Type: Ical.Net.Utility.DateUtil, Method: ToZonedDateTimeLeniently
Type: Ical.Net.Utility.DateUtil, Method: FromTimeZoneToTimeZone
Type: Ical.Net.Utility.DateUtil, Method: FromTimeZoneToTimeZone
Type: Ical.Net.Utility.DateUtil, Method: Truncate
Type: Ical.Net.Utility.DateUtil, Method: WeekOfMonth
Type: Ical.Net.DataTypes.UtcOffset, Method: ToUtc
Type: Ical.Net.DataTypes.UtcOffset, Method: ToLocal
Type: Ical.Net.CalendarComponents.VTimeZone, Method: FromLocalTimeZone
Type: Ical.Net.CalendarComponents.VTimeZone, Method: FromSystemTimeZone
Type: Ical.Net.CalendarComponents.VTimeZone, Method: FromDateTimeZone

Closes #703

axunonb added 2 commits April 7, 2025 10:00
…ction`

Make `EvaluationOptions` nullable

- `CalendarCollection`: Remove `GetOccurrences` overloads using `DaTime`
- `Calendar`: Remove `GetOccurrences` overloads using `DaTime`
- `IGetOccurrences`: enable NRT
- `IGetOccurrences`: enable NRT
- Modified `GetOccurrences` method signatures to accept nullable `CalDateTime` and `EvaluationOptions`
- Adjusted `Evaluate` methods to handle nullable `EvaluationOptions`.
- Updated `Alarm` class methods to utilize nullable options.
@codecov
Copy link

codecov bot commented Apr 7, 2025

Codecov Report

Attention: Patch coverage is 40.00000% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
Ical.Net/CalendarComponents/RecurringComponent.cs 33% 2 Missing ⚠️
Ical.Net/VTimeZoneInfo.cs 50% 1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (40%) is below the target coverage (80%). You can increase the patch coverage or adjust the target coverage.
❌ 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   #761   +/-   ##
===================================
  Coverage    67%    67%           
===================================
  Files       104    104           
  Lines      4517   4508    -9     
  Branches   1106   1103    -3     
===================================
  Hits       3012   3012           
+ Misses     1072   1065    -7     
+ Partials    433    431    -2     
Files with missing lines Coverage Δ
Ical.Net/Calendar.cs 70% <ø> (-<1%) ⬇️
Ical.Net/CalendarCollection.cs 61% <ø> (+4%) ⬆️
Ical.Net/CalendarComponents/Alarm.cs 16% <ø> (ø)
Ical.Net/CalendarComponents/Todo.cs 66% <ø> (ø)
Ical.Net/Evaluation/EventEvaluator.cs 88% <ø> (ø)
Ical.Net/Evaluation/RecurrencePatternEvaluator.cs 92% <ø> (+<1%) ⬆️
Ical.Net/Evaluation/RecurringEvaluator.cs 92% <ø> (ø)
Ical.Net/Evaluation/TimeZoneInfoEvaluator.cs 20% <ø> (+6%) ⬆️
Ical.Net/Evaluation/TodoEvaluator.cs 62% <ø> (ø)
Ical.Net/VTimeZoneInfo.cs 37% <50%> (+1%) ⬆️
... and 1 more

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@axunonb axunonb force-pushed the wip/axunonb/pr/idatetime-2-caldatetime branch from 8c9d01b to 36d505a Compare April 7, 2025 09:23
@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 7, 2025

@axunonb axunonb marked this pull request as ready for review April 7, 2025 09:30
@axunonb axunonb requested review from Copilot and minichma April 7, 2025 09:30
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

Ical.Net/VTimeZoneInfo.cs:100

  • [nitpick] When a null value is assigned to TimeZoneName, an empty string is added instead. Consider whether this behavior aligns with the intended design or if null should clear the list instead.
TimeZoneNames.Add(value ?? string.Empty);

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.

Great. As so often - the best code is deleted code.

{
TimeZoneNames.Clear();
TimeZoneNames.Add(value);
TimeZoneNames.Add(value ?? string.Empty);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looking at the getter, null would represent an empty list rather than a list with an empty string as its sole member.

@axunonb axunonb merged commit a2e8512 into main Apr 7, 2025
6 of 8 checks passed
@axunonb axunonb deleted the wip/axunonb/pr/idatetime-2-caldatetime branch April 7, 2025 19:05
@axunonb axunonb restored the wip/axunonb/pr/idatetime-2-caldatetime branch April 7, 2025 19:05
@axunonb axunonb deleted the wip/axunonb/pr/idatetime-2-caldatetime branch April 13, 2025 07:44
@axunonb axunonb mentioned this pull request May 12, 2025
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.

Replace public methods / properties using DateTime with CalDateTime

3 participants