-
Notifications
You must be signed in to change notification settings - Fork 7
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
V2 #113
Merged
Merged
V2 #113
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Conflicts: # src/api/resources/document/client/Client.ts # src/api/resources/graph/client/Client.ts # src/api/resources/graph/resources/edge/client/Client.ts # src/api/resources/graph/resources/episode/client/Client.ts # src/api/resources/graph/resources/node/client/Client.ts # src/api/resources/group/client/Client.ts # src/api/resources/memory/client/Client.ts # src/api/resources/user/client/Client.ts # src/serialization/types/AddMemoryResponse.ts
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 098b833 in 1 minute and 12 seconds
More details
- Looked at
1365
lines of code in22
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. tests/unit/fetcher/stream-wrappers/webpack.test.ts:3
- Draft comment:
Thedescribe.skip
was removed, enabling this test. Ensure this change is intentional and the test is expected to pass. - Reason this comment was not posted:
Confidence changes required:50%
The PR includes a change in the test filewebpack.test.ts
where adescribe.skip
was removed, enabling the test. This change should be acknowledged as it affects the test execution.
2. reference.md:2683
- Draft comment:
Replacegroup_id
withgroup ID
for consistency and clarity. This applies to other instances in the file as well. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
- The term "group_id" appears in code examples and parameter names, which are technical contexts where underscores are common and appropriate. 2. In natural language descriptions, "group ID" would be more proper English. 3. However, this seems like a minor stylistic preference rather than a real issue. 4. Changing this could actually cause confusion since the code examples use group_id.
The comment has a point about English style, but it could make documentation less consistent with the actual code, potentially confusing users.
While proper English style would favor "group ID", maintaining consistency with code examples is more important for technical documentation.
The comment should be deleted as it suggests a change that could reduce clarity by making documentation inconsistent with code examples.
Workflow ID: wflow_TupdhCUbRLVL6y0k
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Important
Add group management functionalities and update SDK version to 2.3.1, including new methods for listing, retrieving, and getting facts about groups.
listAllGroups
,getAGroup
, andgetFacts
methods toGroup
class insrc/api/resources/group/client/Client.ts
.2.3.1
inpackage.json
,src/version.ts
, andsrc/api/resources/document/client/Client.ts
.GetGroupsOrderedRequest
type insrc/api/resources/group/client/requests/GetGroupsOrderedRequest.ts
.ApidataGroupListResponse
model insrc/api/types/ApidataGroupListResponse.ts
.Group
model to includefactRatingInstruction
andgroupId
insrc/api/types/Group.ts
.ApidataGroupListResponse
serializer insrc/serialization/types/ApidataGroupListResponse.ts
.Group
serializer to includefactRatingInstruction
andgroupId
insrc/serialization/types/Group.ts
..fernignore
to includepackage.json
andyarn.lock
.webpack
test intests/unit/fetcher/stream-wrappers/webpack.test.ts
.This description was created by for 098b833. It will automatically update as commits are pushed.