Skip to content
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

Implemented: predefined and custom option for selecting job run time (#85zrxc0xf) #449

Merged
merged 7 commits into from
May 8, 2023

Conversation

alsoK2maan
Copy link
Contributor

@alsoK2maan alsoK2maan commented Apr 20, 2023

Related Issues

Closes #430

Short Description and Why It's Useful

Screenshots of Visual Changes before/after (If There Are Any)

Screencast.from.24-04-23.03.35.32.PM.IST.webm

IMPORTANT NOTICE - Remember to add changelog entry

Contribution and Currently Important Rules Acceptance

@alsoK2maan alsoK2maan marked this pull request as ready for review April 24, 2023 10:07
src/components/JobConfiguration.vue Outdated Show resolved Hide resolved
src/components/JobConfiguration.vue Outdated Show resolved Hide resolved
updateRunTime(event: CustomEvent) {
const value = event.detail.value
if (value != 'CUSTOM') {
// check for the first 5, predefined options
Copy link
Contributor

Choose a reason for hiding this comment

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

Update comment, why we have done this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated sir

src/components/JobConfiguration.vue Outdated Show resolved Hide resolved
const value = event.detail.value
if (value != 'CUSTOM') {
// checking if a predefined option is selected i.e the first five options
const isPrefinedValue = this.runTimeOptions.slice(0, 5).some((option: any) => option.value === value)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const isPrefinedValue = this.runTimeOptions.slice(0, 5).some((option: any) => option.value === value)
const isPredefinedValue = this.runTimeOptions.slice(0, 5).some((option: any) => option.value === value)

},
// value denotes the time in milliseconds for that label
runTimeOptions: () => [{
"value": 0,
Copy link
Contributor

@adityasharma7 adityasharma7 May 1, 2023

Choose a reason for hiding this comment

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

I think due to this time will be passed to server resulting in schedule failure

value: setTime,
type: 'CUSTOM'
}
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think custom option will remain selected in this case

@adityasharma7 adityasharma7 merged commit 5dfb86c into hotwax:main May 8, 2023
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.

Job Runtime should be selectable from predefined options
4 participants