Skip to content

Conversation

@essweine
Copy link
Contributor

  1. Switch timer definition formats to ISO 8601
  2. Correct behavior of cycle timers

@essweine essweine requested review from danfunk and jbirddog January 19, 2023 13:12
Copy link
Collaborator

@danfunk danfunk left a comment

Choose a reason for hiding this comment

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

Very thorough. Will need to test against CR-Connect to see how it holds up to those files. I do wonder if an existing library could not parse these values and would have examined more case.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 6 Code Smells

93.2% 93.2% Coverage
0.0% 0.0% Duplication

@essweine
Copy link
Contributor Author

essweine commented Jan 20, 2023

I looked at several ISO 8601 parsing libraries, but they were generally missing support for something I needed (recurring intervals most commonly) and they didn't necessarily actually handle time deltas -- you might get something like my parsed_duration, but not an actual timedelta which must be based on an actual time; that was the hard part (the string parsing is not complicated). I found one that had support for recurrences, but it was via a generator (which is a very sensible solution, I'm basically doing the same thing) but if used it, I'd have to serialize it, so that was a hard pass.

@essweine essweine merged commit 7378639 into main Jan 20, 2023
@essweine essweine deleted the feature/improved-timer-events branch January 20, 2023 02:26
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