-
Notifications
You must be signed in to change notification settings - Fork 789
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
Refactor status util function #1137
Conversation
…src/' <!-- ELLIPSIS_HIDDEN --> > [!IMPORTANT] > Refactor task finalization logic by removing `taskIsFinalized` from `utils.ts` and using `statusIsFinalized` in `TaskDetails.tsx`. > > - **Refactor**: > - Remove `taskIsFinalized` from `utils.ts`. > - Replace `taskIsFinalized` with `statusIsFinalized` in `TaskDetails.tsx`. > > <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=Skyvern-AI%2Fskyvern-cloud&utm_source=github&utm_medium=referral)<sup> for 1e93845935e86f8f955b2ad23b5b2b333f08cf93. It will automatically update as commits are pushed.</sup> <!-- ELLIPSIS_HIDDEN -->
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Reviewed everything up to 3c9945e in 11 seconds
More details
- Looked at
49
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. skyvern-frontend/src/routes/tasks/detail/TaskDetails.tsx:34
- Draft comment:
EnsurestatusIsFinalized
is correctly imported from the intended module. Double-check the import path. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_kBb7tDwZUN62SPsr
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Changes requested. Incremental review on 3c9945e in 43 seconds
More details
- Looked at
47
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_iA2lyF9szSn7EcYm
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
@@ -147,7 +147,7 @@ function TaskDetails() { | |||
const taskIsRunningOrQueued = | |||
task?.status === Status.Running || task?.status === Status.Queued; | |||
|
|||
const taskHasTerminalState = task && taskIsFinalized(task); | |||
const taskHasTerminalState = task && statusIsFinalized(task); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
statusIsFinalized
should be called with task.status
instead of task
.
const taskHasTerminalState = task && statusIsFinalized(task); | |
const taskHasTerminalState = task && statusIsFinalized(task.status); |
Important
Refactor task finalization logic by removing
taskIsFinalized
fromutils.ts
and usingstatusIsFinalized
inTaskDetails.tsx
.taskIsFinalized
fromutils.ts
.taskIsFinalized
withstatusIsFinalized
inTaskDetails.tsx
.This description was created by for 3c9945e. It will automatically update as commits are pushed.