-
Notifications
You must be signed in to change notification settings - Fork 5
add batch-async related endpoints #23
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
Conversation
PR SummaryThis pull request adds two new endpoints to the Tasks resource in the PHP Onfleet SDK:
These additions enhance the SDK's capabilities for handling large batch operations in an asynchronous manner, which can improve performance for applications that need to create multiple tasks at once. |
|
I notice you've added two new API endpoints:
These additions represent new application logic that should be verified with unit tests. Adding tests will help ensure these endpoints work as expected and continue to function correctly as the codebase evolves. Could you please add appropriate unit tests for these new endpoints? Thank you! |
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.
Important
Looks good to me! 👍
Reviewed everything up to 99ecdea in 42 seconds. Click for details.
- Reviewed
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/resources/Tasks.php:23
- Draft comment:
Consider aligning async endpoint paths for consistency. The async creation endpoint ('batchCreateAsync') uses '/tasks/batch-async', but its status endpoint ('getBatchCreateAsyncStatus') uses '/tasks/batch/:batchId'. For clarity, consider using a consistent pattern (e.g., '/tasks/batch-async/:batchId'). - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_mcnIufBg6iyjhWOP
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
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.
Important
Looks good to me! 👍
Reviewed 0d9515c in 55 seconds. Click for details.
- Reviewed
26lines of code in2files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. README.es.md:90
- Draft comment:
Se han actualizado los endpoints para Tasks: se añade 'getBatchCreateAsync(id)' a GET y se cambia 'batch(obj)' por 'batchCreate(obj)' junto con la adición de 'batchCreateAsync(obj)' en POST. Asegúrate de que estos cambios reflejen correctamente la implementación de la API. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, describing changes made to the API endpoints. It does not provide any actionable feedback or suggestions for improvement. According to the rules, purely informative comments should be removed.
2. README.md:95
- Draft comment:
Se actualizaron los endpoints de Tasks en la tabla: se añade 'getBatchCreateAsync(id)' en GET y se reemplaza 'batch(obj)' por 'batchCreate(obj)' y se añade 'batchCreateAsync(obj)' en POST. Verifica que estos nombres coincidan con la implementación actual. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to verify that the endpoint names match the current implementation. This falls under the rule of not asking the author to confirm or double-check things. Therefore, this comment should be removed.
Workflow ID: wflow_HzDZtLMfkI3uaZ5E
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
gonzalogrisafi
left a comment
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.
LGTM!
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.
Important
Looks good to me! 👍
Reviewed d013803 in 43 seconds. Click for details.
- Reviewed
16lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. CHANGELOG.md:8
- Draft comment:
Changelog update for version 1.2.0 is clear and follows the established format. The new endpoints 'batchCreateAsync' and 'getBatchCreateAsyncStatus' are documented as expected. Ensure that the implementation, tests, and API docs are updated to match these changelog entries. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking the PR author to ensure that the implementation, tests, and API docs are updated to match the changelog entries. This falls under the rule of not asking the PR author to ensure things are tested or updated, which is not allowed.
Workflow ID: wflow_VLzgOBXivgouKTDd
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Important
Add
batchCreateAsyncandgetBatchCreateAsyncStatusendpoints toTasks.phpfor asynchronous batch task operations, with documentation updates.batchCreateAsyncendpoint for asynchronous batch task creation inTasks.php.getBatchCreateAsyncStatusendpoint for retrieving status of asynchronous batch task creation inTasks.php.CHANGELOG.mdto include new endpoints in version 1.2.0.README.mdandREADME.es.mdto reflect newTasksoperations:batchCreateAsyncandgetBatchCreateAsyncStatus.This description was created by
for d013803. You can customize this summary. It will automatically update as commits are pushed.