Improve tasks and batches types, refactor related tests #1943
Conversation
…s into improve-task-batch
|
@Strift I'm having trouble finding names in the documentation for these types https://www.meilisearch.com/docs/reference/api/tasks. |
WalkthroughThis update refactors all task and batch management API calls in documentation, code samples, and tests to use the new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client
participant Tasks
participant Batches
User->>Client: client.tasks.getTasks(query)
Client->>Tasks: getTasks(query)
Tasks-->>Client: TasksResults
Client-->>User: TasksResults
User->>Client: client.batches.getBatches(query)
Client->>Batches: getBatches(query)
Batches-->>Client: BatchesResults
Client-->>User: BatchesResults
Assessment against linked issues
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/task.ts (1)
109-134:⚠️ Potential issuePotential timeout handle leak – clear the timer in a finally block
clearTimeout(toId)is only invoked on the fast-path where the task leaves the
enqueued|processingstates.
If the polling loop throws for a different reason (network error, 404, etc.),
or if a timeout error is re-thrown, the pending timer is left dangling until
Node’s event loop exits – a minor but avoidable resource leak.- try { + try { for (;;) { … } - } catch (error) { + } catch (error) { … - } + } finally { + if (toId !== undefined) clearTimeout(toId); + }
♻️ Duplicate comments (1)
README.md (1)
466-466: 🛠️ Refactor suggestionUpdate the return type names in documentation to match implementation.
The documentation still references old type names like
EnqueuedTaskbut according to the PR objectives and past review comments, these should be updated to match the new type names likeSummarizedTaskView.-client.index('myIndex').addDocuments(documents: Document<T>[]): Promise<EnqueuedTask> +client.index('myIndex').addDocuments(documents: Document<T>[]): Promise<SummarizedTaskPromise> -client.index('myIndex').deleteAllDocuments(): Promise<Types.EnqueuedTask> +client.index('myIndex').deleteAllDocuments(): Promise<Types.SummarizedTaskPromise> -client.tasks.deleteTasks(parameters: DeleteTasksQuery = {}): Promise<EnqueuedTask> +client.tasks.deleteTasks(parameters: TaskDeletionOrCancelationQuery = {}): Promise<SummarizedTaskPromise>Also applies to: 527-527, 546-546
🧹 Nitpick comments (8)
tests/tasks-and-batches.test.ts (2)
174-175: Improve comment clarityThe comment "this is the only way to get batch uids at the moment of writing this" lacks context. Consider expanding it to explain why batch UIDs are only available through this method and whether this is expected to change in the future.
211-248: Consider adding error case testingWhile the happy path for cancel and delete tasks is well tested, the suite doesn't appear to test error conditions or edge cases with invalid parameters. Consider adding tests for scenarios like:
- Invalid filter values
- Non-existent tasks
- Permissions issues
tests/utils/meilisearch-test-utils.ts (1)
152-153: Consider making the property count check more flexibleThe task validation includes a hard-coded check for 11-12 properties. This approach might be brittle if the
TaskViewtype is updated in the future. Consider using a more adaptable approach that won't break with minor property additions or removals.- vitestAssert(length >= 11 && length <= 12); + // Ensure task has at least all required properties + const requiredProperties = ['indexUid', 'status', 'type', 'enqueuedAt', + 'uid', 'batchUid', 'canceledBy', 'error', + 'duration', 'startedAt', 'finishedAt']; + for (const prop of requiredProperties) { + vitestAssert.property(task, prop); + }src/types/task_and_batch.ts (1)
166-172:Results<T>is public but not exported – consider explicit export
Results<T>looks like a reusable helper for any cursor-style endpoint.
Declaring it astype Results<T>without theexportkeyword keeps it file-local, yet it is referenced by the exported aliasesAllTasksandAllBatches.While TypeScript allows re-export through those aliases, making
Resultsitselfexported improves discoverability and lets downstream users leverage the utility directly.-type Results<T> = { +export type Results<T> = {src/task.ts (2)
143-152:waitForTasksIterprocesses tasks sequentially – consider parallel waitsThe generator awaits each task one after another, which can be slow when many
tasks are in flight. A light refactor can run the polling concurrently and
yield results as soon as each completes, preserving back-pressure while cutting
total wait time.If backwards-compatibility allows,
Promise.allSettledor a simple pool of
waitForTaskpromises would achieve this.
50-56: Minor: inline arrow function reduces readability
getTaskUidis concise but its multi-line ternary coupled with an implicit
return hampers readability. A classicifstatement or single-line arrow
would be clearer.-const getTaskUid = ( - taskUidOrSummarizedTask: TaskUidOrSummarizedTaskView, -): number => - typeof taskUidOrSummarizedTask === "number" - ? taskUidOrSummarizedTask - : taskUidOrSummarizedTask.taskUid; +function getTaskUid( + task: TaskUidOrSummarizedTaskView, +): number { + return typeof task === "number" ? task : task.taskUid; +}src/indexes.ts (2)
242-249: Parameter order ofIndex.createis error-prone
create(uid, options, config)placesconfiglast, while most factory
helpers in the code-base accept(config, …others). The current ordering
makes accidental omission ofoptionseasy and can lead to a runtime “config
undefined” bug.If changing the signature is within scope, prefer
static create(config: Config, uid: string, options: IndexOptions = {})to align with the other constructors.
390-404:addDocumentsInBatchesreturns an array but omits type safety on lengthThe method returns
SummarizedTaskPromise[]but provides no hint about how
many promises correspond to the input array. Consider returning a tuple with a
batchSizefield or exposing a helper thatPromise.alls them to keep the UX
simple.Not blocking, but worth brainstorming for a future DX enhancement.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (15)
.code-samples.meilisearch.yaml(3 hunks)README.md(2 hunks)src/batch.ts(2 hunks)src/indexes.ts(58 hunks)src/meilisearch.ts(10 hunks)src/task.ts(6 hunks)src/types/shared.ts(1 hunks)src/types/task_and_batch.ts(2 hunks)src/types/types.ts(1 hunks)tests/batch.test.ts(0 hunks)tests/task.test.ts(0 hunks)tests/tasks-and-batches.test.ts(1 hunks)tests/utils/meilisearch-test-utils.ts(4 hunks)tests/utils/tasks-and-batches.ts(1 hunks)tests/wait_for_task.test.ts(0 hunks)
💤 Files with no reviewable changes (3)
- tests/batch.test.ts
- tests/wait_for_task.test.ts
- tests/task.test.ts
🧰 Additional context used
🧬 Code Graph Analysis (5)
tests/tasks-and-batches.test.ts (3)
tests/utils/meilisearch-test-utils.ts (2)
getClient(364-364)objectEntries(352-352)src/types/task_and_batch.ts (1)
TasksFilterQuery(63-79)tests/utils/tasks-and-batches.ts (3)
possibleKinds(19-33)possibleStatuses(11-17)assert(162-165)
src/batch.ts (1)
src/types/task_and_batch.ts (3)
BatchView(224-233)TasksFilterQuery(63-79)AllBatches(240-240)
tests/utils/meilisearch-test-utils.ts (3)
src/types/types.ts (1)
MeiliSearchErrorResponse(754-761)src/types/task_and_batch.ts (3)
TaskView(143-152)Status(33-35)Kind(42-56)src/http-requests.ts (1)
T(199-264)
src/types/task_and_batch.ts (3)
src/types/shared.ts (4)
PascalToCamelCase(21-21)OptionStarOrList(27-27)OptionStarOr(25-25)SafeOmit(23-23)src/types/types.ts (2)
Settings(638-668)MeiliSearchErrorResponse(754-761)src/http-requests.ts (1)
T(199-264)
src/task.ts (3)
src/types/task_and_batch.ts (8)
SummarizedTaskView(97-103)SummarizedTaskPromise(158-164)WaitOptions(11-26)TaskView(143-152)TaskUidOrSummarizedTaskView(106-108)TasksFilterQuery(63-79)AllTasks(179-179)TaskDeletionOrCancelationQuery(87-90)src/meilisearch.ts (1)
tasks(48-50)src/http-requests.ts (1)
HttpRequests(140-290)
🔇 Additional comments (24)
src/types/types.ts (1)
79-80: LGTM: Good addition of thedefaultWaitOptionsconfig property.The new optional property enhances the configuration options by allowing users to set default wait options globally, improving the task management experience.
.code-samples.meilisearch.yaml (1)
102-104: API restructuring provides better organization for task and batch methods.All task-related methods (
getTasks,getTask,deleteTasks,cancelTasks) have been moved under thetasksnamespace, and batch-related methods (getBatches,getBatch) under thebatchesnamespace. This creates a more intuitive API grouping that aligns with the internal restructuring.Also applies to: 106-107, 108-112, 114-115, 116-117, 118-120, 122-123, 124-125, 126-127, 128-129, 130-131, 461-462, 809-811
src/types/shared.ts (1)
20-27: Great addition of utility types to enhance type safety.These utility types improve the TypeScript system with:
PascalToCamelCase: Useful for type conversions between different naming conventionsSafeOmit: A well-named alias for the built-in Omit typeOptionStarOrandOptionStarOrList: Flexible type unions that support wildcard optionsThese types will help create more expressive and safer filtering queries for tasks and batches.
README.md (1)
534-535: Update the method access patterns for tasks and batches.The documentation now correctly reflects the new API structure where task-related methods are accessed via
client.tasks.*and batch-related methods viaclient.batches.*, matching the implementation changes.Also applies to: 540-541, 546-547, 552-553, 558-559, 564-565, 584-585, 590-591
src/batch.ts (2)
1-1: Improved type imports to use the new type names.The import statement now uses the renamed types (
BatchView,AllBatches,TasksFilterQuery) that better align with the Meilisearch Rust source code and provide more consistent naming.
17-18: Method signatures updated with improved types and streamlined implementation.The method signatures have been updated to use the new type names and the implementation has been simplified:
getBatchnow returnsPromise<BatchView>(previouslyBatch)getBatchesacceptsTasksFilterQuery(previouslyTasksOrBatchesQuery) and returnsPromise<AllBatches>- The implementation is more concise by directly returning the awaited HTTP request
These changes maintain the same functionality while improving type consistency.
Also applies to: 22-23
tests/tasks-and-batches.test.ts (3)
119-146: Test coverage for task operations looks goodThe tests for
waitForTaskandgetTaskmethods thoroughly verify the task structure and the timeout/interval parameters. Good use of spying on setTimeout to verify that the correct interval and timeout values are passed.
149-165: Comprehensive test for concurrent task waitingThe test implementation for
waitForTaskscorrectly validates that multiple tasks can be waited on concurrently and that the returned tasks have the expected structure.
183-209: Well-designed dynamic test generationThe dynamic generation of test cases using
describe.for()andtest.for()with the filter parameters is an excellent approach. This ensures comprehensive testing of all filter options with minimal code duplication.tests/utils/meilisearch-test-utils.ts (4)
137-143: Good addition of resolve assertionThe addition of a
resolvesassertion is a good complement to the existingrejectsmethod and provides balanced testing capabilities for promise resolution in tests.
144-150: Well-structured error response validationThe
isErrorResponseassertion properly validates the structure of MeiliSearch error responses with appropriate type checks.
171-199: Thorough status and type validationThe validation of task status and type using
objectKeys<Status>andobjectKeys<Kind>withoneOfassertions is a strong approach that ensures values match the defined enums. This aligns well with the type renaming objective of the PR.
342-344: Useful type-safe utility functionThe
objectKeysfunction provides a type-safe way to extract string literal keys from an object type, which is very useful for working with enum-like types in a type-safe manner.src/meilisearch.ts (5)
27-27: Type renaming correctly implementedThe import change from
EnqueuedTaskPromisetoSummarizedTaskPromisealigns with the PR objective to standardize type names.
36-39: Function and type renaming correctly implementedThe updates to import
getHttpRequestsWithSummarizedTaskPromiseand useHttpRequestsWithSummarizedTaskPromisetype are consistent with the PR's renaming strategy.
57-57: Property type correctly updatedThe private property type has been updated to match the renamed type.
74-77: Function name correctly updatedThe function call has been updated to use the renamed function.
152-452: Return types consistently updatedAll method return types (
createIndex,updateIndex,deleteIndex,swapIndexes,createDump,createSnapshot) have been consistently updated to useSummarizedTaskPromise, maintaining type consistency across the codebase.tests/utils/tasks-and-batches.ts (5)
11-33: Well-defined constants for enum valuesThe definition of
possibleStatusesandpossibleKindsas constants derived from type definitions is excellent. This approach ensures that the test utilities stay in sync with the actual type definitions, and provides a source of truth for validating enum values in tests.
36-49: Thorough summarized task validationThe
isSummarizedTaskassertion provides a comprehensive validation of theSummarizedTaskViewtype structure, properly checking all required properties and their types.
51-145: Comprehensive batch validationThe
isBatchassertion is thorough and validates the complex nested structure of batch objects, including proper handling of nullable fields and detailed validation of nested objects.
147-159: Clear result validationThe
isResultassertion properly validates the structure of paginated results, including appropriate checks for nullable fields.
162-165: Smart composition of assertionsThe approach of combining the external assertions with custom assertions creates a unified API that's easy to use in tests. This is a clean pattern that keeps the code organized.
src/types/task_and_batch.ts (1)
42-56: Double-check the literal for the new “UpgradeDatabase” kindGreat to see the new kind being exposed.
The public API currently documents a task type nameddatabaseUpgrade(all lower-camel case). In Rust it is indeed writtenUpgradeDatabase, but the HTTP payload that the JS client receives must match the wire format. Please confirm that Meilisearch actually serialises this variant to the stringupgradeDatabase(lower-camel) before shipping – otherwise the type‐level transformation will mis-match andKindwill accept a value the server never returns.If the payload is
upgradeDatabase, append it to the union and drop"UpgradeDatabase"; if both can appear, accept both at the type level.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
tests/tasks-and-batches.test.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/tasks-and-batches.test.ts (3)
tests/utils/meilisearch-test-utils.ts (2)
getClient(364-364)objectEntries(352-352)src/types/task_and_batch.ts (1)
TasksFilterQuery(63-79)tests/utils/tasks-and-batches.ts (3)
possibleKinds(19-33)possibleStatuses(11-17)assert(162-165)
|
@Strift Should I revert all of the renames? And do the same for the other similar PRs? |
|
Hey @flevi29, I think the I'm okay with renaming internal types, but anything that is exported should remain unchanged for now. I'm mentioning this because I'm happy to merge the other changes in this PR as soon as possible, but we can't prioritize any breaking changes now. |
|
Alright, then I think I'm switching the names back. We'll discuss renaming things later. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/task.ts(2 hunks)tests/tasks-and-batches.test.ts(1 hunks)tests/utils/tasks-and-batches.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/task.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
tests/utils/tasks-and-batches.ts (3)
tests/utils/meilisearch-test-utils.ts (2)
objectKeys(353-353)assert(226-229)src/types/task_and_batch.ts (5)
TaskStatus(33-35)TaskType(42-56)EnqueuedTask(97-103)Batch(222-231)TasksResults(177-177)src/types/shared.ts (1)
SafeOmit(23-23)
tests/tasks-and-batches.test.ts (3)
tests/utils/meilisearch-test-utils.ts (3)
getClient(364-364)assert(226-229)objectEntries(352-352)src/types/task_and_batch.ts (1)
TasksOrBatchesQuery(63-79)tests/utils/tasks-and-batches.ts (3)
possibleTaskTypes(19-33)possibleTaskStatuses(11-17)assert(162-165)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: integration-tests (Node.js 22)
- GitHub Check: integration-tests (Node.js 18)
- GitHub Check: integration-tests (Node.js 20)
🔇 Additional comments (3)
tests/tasks-and-batches.test.ts (1)
1-253: Excellent comprehensive test suite implementation!This is a well-structured consolidation of the previous separate test files. The implementation demonstrates several good practices:
- Comprehensive test coverage using dynamic test generation with
describe.for/test.for- Type-safe test value definitions using
satisfiesclause- Proper use of the new
client.tasksandclient.batchesnamespaces- Custom assertions for validating complex object structures
- Good test isolation with
beforeAllcleanupThe test value coverage is particularly thorough, testing all query parameters including edge cases with wildcard values.
tests/utils/tasks-and-batches.ts (2)
11-33: Well-implemented type-safe constants for test values.The use of
objectKeys<TaskStatus>andobjectKeys<TaskType>provides excellent type safety, ensuring the test constants stay in sync with the actual type definitions. This approach prevents issues when task types or statuses are added or modified.
35-160: Comprehensive and detailed assertion implementations.The custom assertions provide thorough validation of complex nested objects:
isEnqueuedTask: Validates exact object shape and property typesisBatch: Handles complex nested structures with optional fieldsisResult: Validates pagination metadata structureThe assertions appropriately handle nullable fields and provide clear error messages. The validation of batch progress steps and statistics is particularly well-implemented.
flevi29
added
maintenance
|
@Strift Done. |
Co-authored-by: Laurent Cazanove <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/types/task_and_batch.ts (2)
206-211: Good addition of new BatchStats fields with proper documentation.The new optional fields are well-documented with API links. Consider using more specific types instead of
RecordAnyif the structure is known, for better type safety.
227-227: Clarify the purpose of the commented field.The commented
batcherStoppedBecausefield suggests uncertainty about its inclusion. Consider either removing it entirely or adding a comment explaining why it's commented out (e.g., "Reserved for future API version" or "Field exists but type TBD").- // batcherStoppedBecause: unknown; + // batcherStoppedBecause: unknown; // Reserved for future API version
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/types/task_and_batch.ts(6 hunks)tests/utils/tasks-and-batches.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/utils/tasks-and-batches.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: integration-tests (Node.js 22)
- GitHub Check: integration-tests (Node.js 20)
- GitHub Check: integration-tests (Node.js 18)
🔇 Additional comments (7)
src/types/task_and_batch.ts (7)
1-7: LGTM! Good addition of utility types.The new utility type imports from shared.js will improve type safety and flexibility while maintaining API compatibility.
28-35: Excellent solution for the naming convention issue.Using
PascalToCamelCasewrapper maintains API compatibility while aligning with Rust source naming internally. This addresses the discussion about avoiding breaking changes to exported types.
63-79: Excellent improvement in query type flexibility and accuracy.The use of
OptionStarOrandOptionStarOrListutility types better represents the API's wildcard support, and nullable types improve accuracy. This enhances both type safety and developer experience.
87-90: Good use of SafeOmit for improved type safety.Replacing
OmitwithSafeOmitimproves type safety while maintaining the same functionality.
111-134: Good expansion of TaskDetails with upgrade-related fields.The new optional
upgradeFromandupgradeTofields align with the new"UpgradeDatabase"task type. Non-breaking addition with improved documentation.
164-170: Excellent abstraction with generic Results type.The generic
Results<T>type provides good code reuse and consistency betweenTasksResultsandBatchesResults. This unifies the pagination structure and improves maintainability.Also applies to: 177-177, 235-235
37-56:✅ Verification successful
Good consistency with TaskStatus pattern and new task type addition.
The
PascalToCamelCasewrapper maintains consistency. The new"UpgradeDatabase"task type appears to be a valid addition.Please verify that the new
"UpgradeDatabase"task type is properly documented in the Meilisearch API:
🌐 Web query:
Does Meilisearch API support "UpgradeDatabase" task type? What version was it introduced?💡 Result:
Yes, Meilisearch API supports the "UpgradeDatabase" task type. This task type was introduced in Meilisearch version 1.13 as part of the experimental "dumpless upgrade" feature.
The "UpgradeDatabase" task is specifically designed for upgrading Meilisearch databases without the need to create and import dumps, which was previously the only method for upgrading between versions. This task is automatically created when you upgrade from Meilisearch version 1.12 or later to version 1.13 or later using the dumpless upgrade method[1].
How the UpgradeDatabase Task Works
When you perform a dumpless upgrade, the following process occurs:
- You stop your current Meilisearch instance
- Install the new Meilisearch binary
- Relaunch Meilisearch with the same data directory
- Meilisearch automatically creates an "UpgradeDatabase" task[1]
The "UpgradeDatabase" task is processed immediately upon launch and cannot be canceled. You can monitor its progress using the tasks API endpoint:
GET /tasks?types=UpgradeDatabaseThis will return the task UID, which you can then use to check the specific task status:
GET /tasks/TASK_UIDWhile the upgrade task is processing, you can still make search queries, and you can even enqueue new tasks. However, Meilisearch will only process these new tasks after the "UpgradeDatabase" task is completed[1].
Limitations and Considerations
The dumpless upgrade feature is experimental and only works when upgrading from Meilisearch version 1.12 or later to version 1.13 or later[1]. For earlier versions, you must still use the dump-based migration process.
Before performing a dumpless upgrade, it's recommended to create a snapshot of your instance as a backup in case the upgrade process fails or results in a corrupted database[1].
Kyethen 1.14, Meilisearch continued to develop its task system with additional features like more granular progress levels for measuring merging operations and displaying database sizes in the batch route[9].
Citations:
- 1: https://www.meilisearch.com/docs/learn/update_and_migration/updating
- 2: https://www.meilisearch.com/docs/learn/async/working_with_tasks
- 3: https://www.meilisearch.com/blog/update-meilisearch-digitalocean-aws-gcp
- 4: Refactoring the task queue and scheduler meilisearch#2725
- 5: https://www.meilisearch.com/docs/reference/api/settings
- 6: Technical overview on how we can auto-update the search engine meilisearch#2570
- 7: https://github.com/meilisearch/open-api/blob/main/open-api.json
- 8: https://specs.meilisearch.dev/specifications/text/0105-dumps-api.html
- 9: https://github.com/meilisearch/meilisearch/releases
- 10: https://www.meilisearch.com/llms-full.txt
Approve TaskType update with “UpgradeDatabase”
Verified that the
UpgradeDatabasetask type was introduced in Meilisearch v1.13 as part of the dumpless upgrade feature and is supported by the API. The change in src/types/task_and_batch.ts (lines 37–56) aligns with the documented behavior.
- src/types/task_and_batch.ts (37–56):
TaskTypenow includes"UpgradeDatabase", matching Meilisearch API v1.13+
There was a problem hiding this comment.
I love what you did here.
Only quirk is that I find the tests output a bit hard to read. We currently have:
✓ tests/tasks-and-batches.test.ts (105 tests) 1261ms
✓ waitForTask and getTask methods 648ms
✓ waitForTasks method 84ms
✓ getBatch method 20ms
✓ limit > getTasks method 2ms
✓ limit > getBatches method 2ms
✓ from > getTasks method 2ms
✓ from > getBatches method 2ms
✓ reverse > getTasks method 2ms
✓ reverse > getBatches method 3ms
✓ batchUids > getTasks method 1ms
✓ batchUids > getTasks method with * 2ms
✓ batchUids > getBatches method 1ms
✓ batchUids > getBatches method with * 2ms
(...)So, the pattern is currently: "specific field" > "method name (+ with special case)".
I think it would read better if it were: "method name" > "specific field (+ with special case)"
But I'm not sure there's a good way to do this without duplicating or complexifying the code, so we can leave it as is. I just mention it in case you have some magic fix in mind, but don't spend time on this otherwise
There was a problem hiding this comment.
I'll think about it, but right now it doesn't seem like a straight forward thing to do. But I'll try to make it more readable however I can in the future.
github-merge-queue
bot
removed this pull request from the merge queue due to a conflict with the base branch
|
It says "This branch cannot be rebased due to conflicts". It would be nice if it would display those conflicts. |
|
Closing in favor of #1952 |
Pull Request
What does this PR do?
Summary by CodeRabbit
Documentation
tasksandbatchesnamespaces on the client.New Features
Bug Fixes
Tests