chore: make streamed list objects client methods private#264
chore: make streamed list objects client methods private#264SoulPancake merged 3 commits intomainfrom
Conversation
WalkthroughThe PR converts public streaming API methods Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
|
Documentation Updates 1 document(s) were updated by changes in this PR: OpenFGA's Space |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #264 +/- ##
=======================================
Coverage 33.89% 33.89%
=======================================
Files 114 114
Lines 11022 11022
=======================================
Hits 3736 3736
Misses 6922 6922
Partials 364 364 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR makes the StreamedListObjects and StreamedListObjectsExecute client methods private by renaming them to lowercase variants (streamedListObjects and streamedListObjectsExecute), removing them from the public SdkClient interface, and updating all internal references and tests accordingly.
- The methods are made private in the client layer
- The example code is preserved in comments with explanatory notes
- All tests are updated to use the new private method names
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| client/client.go | Renamed StreamedListObjects and StreamedListObjectsExecute to lowercase (private) versions and removed them from the SdkClient interface |
| client/streaming_test.go | Updated all test calls to use the new private method names (streamedListObjects) |
| example/streamed_list_objects/main.go | Commented out the example code with explanatory notes about the methods being private |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
client/client.go (2)
3339-3344: Consider adding a godoc comment explaining the internal status.The method is now unexported, but there's no documentation explaining:
- Why this is internal-only
- Whether it's intended to become public in the future
- What alternatives users should use
Consider adding a comment like:
// streamedListObjects is an internal method for streaming list objects. // This API is not yet stabilized and may change in future releases. // Use ListObjects for the stable paginated API. func (client *OpenFgaClient) streamedListObjects(ctx _context.Context) SdkClientStreamedListObjectsRequestInterface {
3286-3304: Public types remain but lack a public entry point.The types
SdkClientStreamedListObjectsRequest,SdkClientStreamedListObjectsRequestInterface,ClientStreamedListObjectsRequest,ClientStreamedListObjectsOptions, andClientStreamedListObjectsResponseare still exported (public), but there's no public method to use them with.This creates an inconsistent API surface. Consider either:
- Making these types internal as well (lowercase) if they're only for internal use
- Documenting why they remain public (e.g., for future public API)
- Adding a comment indicating they're reserved for future use
example/streamed_list_objects/main.go (1)
1-8: Reconsider the approach of maintaining a disabled example file.Rather than keeping a commented-out example, consider one of these alternatives:
- Remove the example file entirely until the API is made public
- Move it to a documentation or design folder as a reference implementation
- Use Go's build tags or file suffix (e.g.,
main.go.disabled) to exclude it from buildsAn empty
main()function in the examples directory serves no purpose for users. If thestreamedListObjectsAPI will eventually become public, preserve the full implementation in a version control history or design document rather than as a disabled file in the active codebase.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
client/client.go(3 hunks)client/streaming_test.go(6 hunks)example/streamed_list_objects/main.go(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: Build, Test and Publish
example/streamed_list_objects/main.go
[error] 1-1: gofmt formatting check failed. The following files are not formatted: example/streamed_list_objects/main.go. Run 'gofmt -w .' to fix.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (3)
client/streaming_test.go (1)
59-59: LGTM! Test updates are consistent.All test cases have been correctly updated to call the internal
streamedListObjectsmethod. The tests maintain their structure and continue to validate the streaming functionality appropriately.Also applies to: 118-118, 165-165, 190-190, 233-233, 300-300
client/client.go (2)
3371-3371: Verify consistency: Execute() delegates to internal method.The change correctly updates the internal delegation from
StreamedListObjectsExecutetostreamedListObjectsExecute. This maintains consistency with the method renaming.
3386-3440: Implementation correctly made internal.The
streamedListObjectsExecutemethod is now properly unexported and maintains all its functionality. The implementation looks correct.
This reverts commit 944ecd8.
Description
What problem is being solved?
How is it being solved?
What changes are made to solve it?
References
Review Checklist
mainSummary by CodeRabbit
Breaking Changes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.