Skip to content

Conversation

@axunonb
Copy link
Collaborator

@axunonb axunonb commented May 10, 2025

Resolves #788

@codecov
Copy link

codecov bot commented May 10, 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   #789   +/-   ##
===================================
  Coverage    67%    67%           
===================================
  Files       106    106           
  Lines      4445   4450    +5     
  Branches   1071   1072    +1     
===================================
+ Hits       2960   2965    +5     
  Misses     1046   1046           
  Partials    439    439           
Files with missing lines Coverage Δ
Ical.Net/Constants.cs 45% <ø> (ø)
Ical.Net/DataTypes/RecurrencePattern.cs 76% <100%> (+2%) ⬆️
Ical.Net/Evaluation/Evaluator.cs 95% <ø> (-5%) ⬇️
Ical.Net/Evaluation/RecurrencePatternEvaluator.cs 91% <100%> (ø)
...alization/DataTypes/RecurrencePatternSerializer.cs 93% <100%> (+1%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

/// - 1 = every
/// - 2 = every second
/// - 3 = every third
/// Specifies the interval between occurrences of the recurrence. The default value is 1.
Copy link
Collaborator

Choose a reason for hiding this comment

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

That's not fully precise, as due to the other rule parts, more or less occurrences could happen per interval. This is what the RFC says, maybe just copy?

The INTERVAL rule part contains a positive integer representing at
which intervals the recurrence rule repeats. The default value is
"1", meaning every second for a SECONDLY rule, every minute for a
MINUTELY rule, every hour for an HOURLY rule, every day for a
DAILY rule, every week for a WEEKLY rule, every month for a
MONTHLY rule, and every year for a YEARLY rule. For example,
within a DAILY rule, a value of "8" means every eight days.

@axunonb axunonb marked this pull request as ready for review May 10, 2025 15:24
@axunonb axunonb requested a review from minichma May 10, 2025 15:24
minichma
minichma previously approved these changes May 10, 2025
default:
throw new Exception("FrequencyType.NONE cannot be evaluated. Please specify a FrequencyType before evaluating the recurrence.");
// Frequency should always be valid at this stage.
System.Diagnostics.Debug.Fail($"'{pattern.Frequency}' as RecurrencePattern.Frequency is not implemented.");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure we should fail here. Users could easily set any int as frequency type, which would not be an internal error. I think throwing was not wrong.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

... kike with (FrequencyType) 123 - yes, you're right. Wouldn't checking for valid values in the setter of RecurrencePattern.Frequency be better?

@axunonb axunonb force-pushed the wip/axunonb/pr/freq-type branch from 22081fc to 84cfd4d Compare May 10, 2025 20:53
@axunonb axunonb requested a review from minichma May 10, 2025 20:54
@sonarqubecloud
Copy link

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.

Handling of FREQ in recurrence pattern

3 participants