-
Notifications
You must be signed in to change notification settings - Fork 234
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
TBD: Make parsing of CalDateTime strings more restrictive #734
base: main
Are you sure you want to change the base?
Conversation
… suffix or are date-only.
|
Codecov ReportAttention: Patch coverage is
❌ Your patch check has failed because the patch coverage (43%) is below the target coverage (80%). You can increase the patch coverage or adjust the target coverage. @@ Coverage Diff @@
## main #734 +/- ##
===================================
- Coverage 66% 66% -0%
===================================
Files 103 103
Lines 4644 4650 +6
Branches 1152 1155 +3
===================================
+ Hits 3064 3066 +2
- Misses 1144 1146 +2
- Partials 436 438 +2
|
First of all: These changes are useful and should get merged. Deserialization of non-standard input: |
A Validate method would come with quite some overhead, because it would validate a string, not a So yes, I think, being more strict would probably be the right way to go but we must be aware that there will be people complaining. Generally speaking, a desirable solution would always be to parse the user content as long as it is syntactically correct and only fail later on during an explicit Validation or during the Evaluation step. But this doesn't work with our new approach to |
TBD: This PR makes the parsing of DATE/DATE-TIME values more restrictive, i.e. rejects if
Z
suffix are presentDATE
, i.e. date-onlyThis needs to be discussed as lib users may run into issues when parsing non-compliant calendars. On the other hand users might not notice that TZIDs have been specified that are being ignored because of the value type
DATE