Conversation
|
Thanks for submitting your first pull request! You are awesome! 🤗 |
for more information, see https://pre-commit.ci
|
|
src/util/job-name-validation.tsx
Outdated
| const jobNameRegex = /^[a-zA-Z0-9._][a-zA-Z0-9._ -]{0,62}$/; | ||
| const invalidFirstCharRegex = /^[^a-zA-Z0-9._]/; | ||
| const invalidCharRegex = /[^a-zA-Z0-9._ -]/g; | ||
| const validIntegerRregex = /^[+]?[1-9]\d*$/; |
There was a problem hiding this comment.
Rregex is misspelled.
I don't see a reason why zero-prefixed integers like 035 should be rejected, since they'll resolve to valid numeric values anyway. The + is redundant in this case. This could be simplified to /^\d+$/.
| const handleNumericInputChange = (event: ChangeEvent<HTMLInputElement>) => { | ||
| const target = event.target; | ||
|
|
||
| const parameterNameIdx = parameterNameMatch(target.name); |
There was a problem hiding this comment.
This code is copied from the code for setting parameters, which have user-provided names and values. Your change is intended to add input fields with only user-provided values.
src/mainviews/create-job.tsx
Outdated
| name="maxRetryAttempts" | ||
| /> | ||
| <TextField | ||
| label={trans.__('Max Run Time (In Seconds)')} |
There was a problem hiding this comment.
| label={trans.__('Max Run Time (In Seconds)')} | |
| label={trans.__('Maximum Run Time (seconds)')} |
"Maximum" was not abbreviated above
src/mainviews/create-job.tsx
Outdated
| name="maxRunTime" | ||
| /> | ||
| <TextField | ||
| label={trans.__('Max Wait Time (In Seconds)')} |
There was a problem hiding this comment.
| label={trans.__('Max Wait Time (In Seconds)')} | |
| label={trans.__('Maximum Wait Time (seconds)')} |
|
|
||
| export function MaxRetryAttemptsError(maxRetryAttempts: string, trans: TranslationBundle): string { | ||
| // Check for blank | ||
| if (maxRetryAttempts === '') { |
There was a problem hiding this comment.
This should be an optional parameter, since existing users editing job definitions will not have these values set.
|
|
||
| if (!validIntegerRregex.test(maxWaitTime)) { | ||
| return trans.__( | ||
| 'Max wait time must be an integer' |
There was a problem hiding this comment.
| 'Max wait time must be an integer' | |
| 'Must be an integer, %1 or greater', minWaitTime |
| const integerValue = +maxWaitTime | ||
| // Check for length. | ||
| if (integerValue < 1) { | ||
| return trans.__('Max wait time must be greater than 1'); |
There was a problem hiding this comment.
| return trans.__('Max wait time must be greater than 1'); | |
| return trans.__('Must be an integer, %1 or greater', minWaitTime) |
| } | ||
| const integerValue = +maxWaitTime | ||
| // Check for length. | ||
| if (integerValue < 1) { |
There was a problem hiding this comment.
Use a symbolic constant
| ); | ||
| } | ||
| const integerValue = +maxWaitTime | ||
| // Check for length. |
There was a problem hiding this comment.
Fix or delete this comment
| ); | ||
| } | ||
| const integerValue = +maxRunTime | ||
| // Check for length. |
There was a problem hiding this comment.
Fix or delete this comment
3 participants
(Added by @JasonWeill: This fixes #169.)