-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat(bulkrun): run mutiple jobs in one req #2
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe recent changes introduce a new type definition and a method within the Changes
Poem
TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
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.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (1)
src/index.ts (1)
65-65
: Add a comment to explain the method.Consider adding a comment to explain the purpose of the
triggerBulkRun
method.+ /** + * Triggers multiple jobs in one request. + * @param args - The arguments for triggering bulk run. + * @returns A promise that resolves to the job data. + */
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/index.ts (2 hunks)
Additional comments not posted (1)
src/index.ts (1)
41-44
: Type definitionTriggerBulkRunArgsType
looks good.The type definition is clear and aligns with the intended functionality.
|
||
// triggers multiple jobs in one request | ||
triggerBulkRun(args: TriggerTaskArgsType): Promise<JobDataType> { | ||
if (!args.taskId) return Promise.reject("taskId is required."); |
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.
Enhance error message.
The error message should be more descriptive to help with debugging.
- if (!args.taskId) return Promise.reject("taskId is required.");
+ if (!args.taskId) return Promise.reject(new Error("taskId is required for triggering bulk run."));
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
if (!args.taskId) return Promise.reject("taskId is required."); | |
if (!args.taskId) return Promise.reject(new Error("taskId is required for triggering bulk run.")); |
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.
shorten the message
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.
@mGolestan98, how about shortening the message to "taskId is required for bulk run."? This keeps it concise while still providing context.
if (!args.taskId) return Promise.reject(new Error("taskId is required for bulk run."));
Let me know if this works for you!
triggerBulkRun(args: TriggerTaskArgsType): Promise<JobDataType> { | ||
if (!args.taskId) return Promise.reject("taskId is required."); | ||
|
||
return this.RequestHandler.post(`/tasks/${args.taskId}/jobs`, { | ||
variables: args.variables | ||
}); | ||
} |
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.
Use the correct type for the method argument.
The argument type for triggerBulkRun
should be TriggerBulkRunArgsType
instead of TriggerTaskArgsType
.
- triggerBulkRun(args: TriggerTaskArgsType): Promise<JobDataType> {
+ triggerBulkRun(args: TriggerBulkRunArgsType): Promise<JobDataType> {
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
triggerBulkRun(args: TriggerTaskArgsType): Promise<JobDataType> { | |
if (!args.taskId) return Promise.reject("taskId is required."); | |
return this.RequestHandler.post(`/tasks/${args.taskId}/jobs`, { | |
variables: args.variables | |
}); | |
} | |
triggerBulkRun(args: TriggerBulkRunArgsType): Promise<JobDataType> { | |
if (!args.taskId) return Promise.reject("taskId is required."); | |
return this.RequestHandler.post(`/tasks/${args.taskId}/jobs`, { | |
variables: args.variables | |
}); | |
} |
@coderabbitai generate interesting stats about this repository and render them as a table. |
Tip For best results, initiate chat on the files or code changes. @mGolestan98, here are the interesting statistics about the repository presented in a table format:
Feel free to ask if you need any additional information or further analysis! Analysis chainScripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Count the number of authors in the codebase.
authors_count=$(git shortlog -sn --all | wc -l)
# Count the number of lines of code in the codebase.
loc_count=$(tokei --output json | jq '.total.lines')
# Find all occurrences of the `TODO` keyword.
todo_count=$(rg 'TODO' | wc -l)
# Output the results as a table.
echo -e "Statistic\tCount"
echo -e "---------\t-----"
echo -e "Authors\t$authors_count"
echo -e "Lines of Code\t$loc_count"
echo -e "TODO Comments\t$todo_count"
Length of output: 365 Script: #!/bin/bash
# Count the number of authors in the codebase.
authors_count=$(git shortlog -sn --all | wc -l)
# Count the number of lines of code in the codebase using wc -l as a fallback.
loc_count=$(find . -name '*.ts' -o -name '*.js' -o -name '*.jsx' -o -name '*.tsx' | xargs wc -l | tail -n 1 | awk '{print $1}')
# Find all occurrences of the `TODO` keyword.
todo_count=$(rg 'TODO' | wc -l)
# Output the results as a table.
echo -e "Statistic\tCount"
echo -e "---------\t-----"
echo -e "Authors\t$authors_count"
echo -e "Lines of Code\t$loc_count"
echo -e "TODO Comments\t$todo_count"
Length of output: 441 |
a8e0846
to
723d7ff
Compare
52b25c8
to
2ccf228
Compare
Summary by CodeRabbit