Merge Sequence Editors Time library #1324
Conversation
There was a problem hiding this comment.
Any reason to have a separate time-utils file when we already have a time utils?
There was a problem hiding this comment.
No real reason beside not knowing about the time utils. Also, this is sequence specific times which has its own set of time tag rules.
There was a problem hiding this comment.
Yeah, that was one of my requests in the sequence ticket. Could the exported functions in this file potentially be used outside of the sequence context?
There was a problem hiding this comment.
They can I need to look at the other time utils and see what I can do.
|
|
||
| export const RELATIVE_TIME = /([0-9]{3}T)?([0-9]{2}):([0-9]{2}):([0-9]{2})(\.[0-9]+)?$/g; | ||
| export const RELATIVE_SIMPLE = /(\d+)(\.[0-9]+)?$/g; | ||
| export enum TimeTypes { |
There was a problem hiding this comment.
there is a time.ts enum file where this could probably be placed for easier discoverability
|
|
||
| export const EPOCH_TIME = /(^[+-]?)([0-9]{3}T)?([0-9]{2}):([0-9]{2}):([0-9]{2})(\.[0-9]+)?$/g; | ||
| export const EPOCH_SIMPLE = /(^[+-]?)(\d+)(\.[0-9]+)?$/g; | ||
| export const RELATIVE_TIME = /^([0-9]{3}T)?([0-9]{2}):([0-9]{2}):([0-9]{2})(\.[0-9]+)?$/; |
There was a problem hiding this comment.
Is it necessary for other places to be using these regexs? Could the testTime function be repurposed to instead of taking the regex as a parameter, take the TimeType as a parameter to be used in combination with the new getTimeRegex function?
| @@ -13,7 +13,7 @@ export const sequenceProvideCodeActions = ( | |||
| .map(unbalancedTime => { | |||
| const match = unbalancedTime.message.match(/Suggestion:\s*(.*)/); | |||
| if (match) { | |||
| const extractSuggestedTime = match[1].replace(/\s+/g, ''); | |||
| const extractSuggestedTime = match[1].replace(/\s+/, '').replace(/\[|\]/g, ''); | |||
There was a problem hiding this comment.
Can this be combined to match[1].replace(/\s+|[\[|\]]/g, '')?
* For string arguments * For descriptions
This was causing the custom worker to incorrectly label the tree nodes and break the custom checks.
* Fix regex and remove 'g' * Added TimeTypes enum instead of passing regex into the utils class * Add doc info
* Uses the update time-utils 'TimeType' enum for the checks
* Use the regex instead of the `testTime` function
* The time check in the custom worker is using the time-utils for its time validation. * Remove duplicate/obsolete code * Updated the Error Codes in the custom worker to match the seqN.txt thus make the whole Aerie sequencing suite uniform
Not sure why these errors are appearing as I didn't touch these files???
* Add sequence related utilities function to the library
goetzrrGit
force-pushed
the
refactor/sequence-time-utils
branch
from
b0b450f to
ff76c00
Compare
goetzrrGit
had a problem deploying
to
test-workflow
GitHub Actions
Failure
|
Ditching this pr for a newer one |
Closes #1318
The
eDSLandSeqNeditors previously used separate time libraries with overlapping functionality. To streamline code and improve maintainability, I've merged them into a single, unified library.Note: Ignore the first 3 commits. They will fall-off when I rebase.