refactor(sdk): move public facade to sdk path - #11
Conversation
|
🧙 Sourcery 已完成对你的拉取请求的审查! 提示与命令与 Sourcery 交互
自定义你的体验访问你的控制台以:
获取帮助Original review guide in English🧙 Sourcery has finished reviewing your pull request! Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthrough新增顶层 Changes公共 SDK
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant SDKClient
participant AppAPI
participant FileSystem
CLI->>SDKClient: DownloadMovieMedia(movieID, options)
SDKClient->>SDKClient: 校验媒体选择和输出路径
SDKClient->>AppAPI: MovieDetail(movieID)
AppAPI-->>SDKClient: 影片媒体 URL
SDKClient->>AppAPI: 下载选定媒体
AppAPI-->>SDKClient: 媒体字节
SDKClient->>FileSystem: 写入媒体文件
FileSystem-->>SDKClient: 输出路径和字节数
SDKClient-->>CLI: MovieMediaDownloadResult
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
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 |
There was a problem hiding this comment.
Hey - 我已经审查了你的更改,一切看起来都很棒!
帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈不断改进代码审查质量。
Original comment in English
Hey - I've reviewed your changes and they look great!
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
sdk/movie_test.go (1)
8-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win覆盖
thumb_url回退路径。当第一张预览图没有
large_url时,movieMediaURLs应返回同一项的thumb_url。当前测试只覆盖large_url成功的路径,无法防止该回退逻辑回归。建议的测试
+func TestMovieMediaURLsFallsBackToFirstPreviewThumbnail(t *testing.T) { + sources := movieMediaURLs(map[string]any{ + "preview_images": []any{ + map[string]any{"thumb_url": "https://media.example.test/first-thumb.jpg"}, + map[string]any{"large_url": "https://media.example.test/second-large.jpg"}, + }, + }) + if sources.previewImage != "https://media.example.test/first-thumb.jpg" { + t.Fatalf("preview image = %q", sources.previewImage) + } +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/movie_test.go` around lines 8 - 29, 在 TestMovieMediaURLsUsesOnlyFirstPreviewImage 中补充第一张预览图缺少 large_url、但包含 thumb_url 的场景,并断言 movieMediaURLs 返回该同一项的 thumb_url 作为 previewImage;保留现有缩略图、视频及不选取后续预览图的断言。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/test-architecture.sh`:
- Around line 16-24: Update the ripgrep check in the architecture test to
capture its exit status explicitly. Treat status 1 as the expected “no matches”
result, but exit with an error for every other nonzero status, including scan
failures or an unavailable rg command; preserve the existing failure for
detected retired SDK imports.
In `@sdk/client.go`:
- Around line 106-117: Propagate each public SDK method’s context.Context
through the full request stack instead of discarding it. In sdk/client.go lines
106-117, update Login and ResolveUserID; in sdk/search.go lines 16-18, Search;
in sdk/browse.go lines 14-34, taxonomy and Browse methods; in sdk/entity.go
lines 13-33, entity queries and pagination aggregation; in sdk/lists.go lines
6-20, list requests; in sdk/rankings.go lines 13-33, ranking requests; and in
sdk/user.go lines 6-38, user reads and mutations. Add context-aware appapi and
HTTP methods, constructing requests with http.NewRequestWithContext or an
equivalent mechanism so cancellation and deadlines reach the underlying
requests.
In `@sdk/movie.go`:
- Around line 27-42: Propagate the caller’s context instead of discarding it
throughout the SDK movie flows: update the appapi request methods used by
MovieDetail, MovieMagnets, MovieComments, and the other affected methods, plus
DownloadImage, fetchMedia, and the HLS download path, to accept and pass context
into their HTTP requests and downloads. Ensure cancellation and deadlines stop
all corresponding network operations, and remove the `_ = ctx` placeholders.
---
Nitpick comments:
In `@sdk/movie_test.go`:
- Around line 8-29: 在 TestMovieMediaURLsUsesOnlyFirstPreviewImage 中补充第一张预览图缺少
large_url、但包含 thumb_url 的场景,并断言 movieMediaURLs 返回该同一项的 thumb_url 作为
previewImage;保留现有缩略图、视频及不选取后续预览图的断言。
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a39211b5-12ce-41f2-8c1e-3eac2738c193
📒 Files selected for processing (40)
.github/copilot-instructions.mdAGENTS.mdCONTRIBUTING.mdCONTRIBUTING.zh-CN.mdREADME.mdREADME.zh-CN.mddocs/en/sdk.mddocs/maintainers/agents/documentation-guidelines.mddocs/maintainers/agents/review-checklist.mddocs/maintainers/architecture.mddocs/maintainers/development.mddocs/zh-CN/sdk.mdinternal/cli/authclient.gointernal/cli/authclient_test.gointernal/cli/comments_cmd.gointernal/cli/detail_cmd.gointernal/cli/download_cmd.gointernal/cli/entity_cmd.gointernal/cli/lists_cmd.gointernal/cli/magnets_cmd.gointernal/cli/rankings_cmd.gointernal/cli/root.gointernal/cli/search_cmd.gointernal/cli/tags_browse_cmd.gointernal/cli/user_cmd.gointernal/cli/user_cmd_test.goscripts/test-architecture.shscripts/test-documentation.shsdk/browse.gosdk/client.gosdk/client_test.gosdk/entity.gosdk/errors.gosdk/lists.gosdk/magnets.gosdk/movie.gosdk/movie_test.gosdk/rankings.gosdk/search.gosdk/user.go
| if rg -n -F 'github.com/FlanChanXwO/javdb-cli/javdb' \ | ||
| "$repo_root/cmd" \ | ||
| "$repo_root/internal" \ | ||
| "$repo_root/sdk" \ | ||
| -g '*.go'; then | ||
| printf '%s\n' 'source code imports the retired public SDK path' >&2 | ||
| exit 1 | ||
| fi | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
区分“无匹配”和扫描错误。
rg 在没有匹配时返回 1,在路径错误或命令不可用时返回其他非零状态。当前命令位于 if 条件中,因此 set -e 不会处理这些错误。架构检查可能在未完成扫描时通过。
请显式保存退出码。只接受 1 作为“未找到”,并对其他非零状态退出。
建议的退出码处理
if rg -n -F 'github.com/FlanChanXwO/javdb-cli/javdb' \
"$repo_root/cmd" \
"$repo_root/internal" \
"$repo_root/sdk" \
-g '*.go'; then
printf '%s\n' 'source code imports the retired public SDK path' >&2
exit 1
+else
+ status=$?
+ if [ "$status" -ne 1 ]; then
+ exit "$status"
+ fi
fi📝 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 rg -n -F 'github.com/FlanChanXwO/javdb-cli/javdb' \ | |
| "$repo_root/cmd" \ | |
| "$repo_root/internal" \ | |
| "$repo_root/sdk" \ | |
| -g '*.go'; then | |
| printf '%s\n' 'source code imports the retired public SDK path' >&2 | |
| exit 1 | |
| fi | |
| if rg -n -F 'github.com/FlanChanXwO/javdb-cli/javdb' \ | |
| "$repo_root/cmd" \ | |
| "$repo_root/internal" \ | |
| "$repo_root/sdk" \ | |
| -g '*.go'; then | |
| printf '%s\n' 'source code imports the retired public SDK path' >&2 | |
| exit 1 | |
| else | |
| status=$? | |
| if [ "$status" -ne 1 ]; then | |
| exit "$status" | |
| fi | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/test-architecture.sh` around lines 16 - 24, Update the ripgrep check
in the architecture test to capture its exit status explicitly. Treat status 1
as the expected “no matches” result, but exit with an error for every other
nonzero status, including scan failures or an unavailable rg command; preserve
the existing failure for detected retired SDK imports.
Source: Coding guidelines
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
🧹 Nitpick comments (1)
sdk/movie_test.go (1)
8-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win覆盖
thumb_url回退路径。当第一张预览图没有
large_url时,movieMediaURLs应返回同一项的thumb_url。当前测试只覆盖large_url成功的路径,无法防止该回退逻辑回归。建议的测试
+func TestMovieMediaURLsFallsBackToFirstPreviewThumbnail(t *testing.T) { + sources := movieMediaURLs(map[string]any{ + "preview_images": []any{ + map[string]any{"thumb_url": "https://media.example.test/first-thumb.jpg"}, + map[string]any{"large_url": "https://media.example.test/second-large.jpg"}, + }, + }) + if sources.previewImage != "https://media.example.test/first-thumb.jpg" { + t.Fatalf("preview image = %q", sources.previewImage) + } +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/movie_test.go` around lines 8 - 29, 在 TestMovieMediaURLsUsesOnlyFirstPreviewImage 中补充第一张预览图缺少 large_url、但包含 thumb_url 的场景,并断言 movieMediaURLs 返回该同一项的 thumb_url 作为 previewImage;保留现有缩略图、视频及不选取后续预览图的断言。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/test-architecture.sh`:
- Around line 16-24: Update the ripgrep check in the architecture test to
capture its exit status explicitly. Treat status 1 as the expected “no matches”
result, but exit with an error for every other nonzero status, including scan
failures or an unavailable rg command; preserve the existing failure for
detected retired SDK imports.
In `@sdk/client.go`:
- Around line 106-117: Propagate each public SDK method’s context.Context
through the full request stack instead of discarding it. In sdk/client.go lines
106-117, update Login and ResolveUserID; in sdk/search.go lines 16-18, Search;
in sdk/browse.go lines 14-34, taxonomy and Browse methods; in sdk/entity.go
lines 13-33, entity queries and pagination aggregation; in sdk/lists.go lines
6-20, list requests; in sdk/rankings.go lines 13-33, ranking requests; and in
sdk/user.go lines 6-38, user reads and mutations. Add context-aware appapi and
HTTP methods, constructing requests with http.NewRequestWithContext or an
equivalent mechanism so cancellation and deadlines reach the underlying
requests.
In `@sdk/movie.go`:
- Around line 27-42: Propagate the caller’s context instead of discarding it
throughout the SDK movie flows: update the appapi request methods used by
MovieDetail, MovieMagnets, MovieComments, and the other affected methods, plus
DownloadImage, fetchMedia, and the HLS download path, to accept and pass context
into their HTTP requests and downloads. Ensure cancellation and deadlines stop
all corresponding network operations, and remove the `_ = ctx` placeholders.
---
Nitpick comments:
In `@sdk/movie_test.go`:
- Around line 8-29: 在 TestMovieMediaURLsUsesOnlyFirstPreviewImage 中补充第一张预览图缺少
large_url、但包含 thumb_url 的场景,并断言 movieMediaURLs 返回该同一项的 thumb_url 作为
previewImage;保留现有缩略图、视频及不选取后续预览图的断言。
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a39211b5-12ce-41f2-8c1e-3eac2738c193
📒 Files selected for processing (40)
.github/copilot-instructions.mdAGENTS.mdCONTRIBUTING.mdCONTRIBUTING.zh-CN.mdREADME.mdREADME.zh-CN.mddocs/en/sdk.mddocs/maintainers/agents/documentation-guidelines.mddocs/maintainers/agents/review-checklist.mddocs/maintainers/architecture.mddocs/maintainers/development.mddocs/zh-CN/sdk.mdinternal/cli/authclient.gointernal/cli/authclient_test.gointernal/cli/comments_cmd.gointernal/cli/detail_cmd.gointernal/cli/download_cmd.gointernal/cli/entity_cmd.gointernal/cli/lists_cmd.gointernal/cli/magnets_cmd.gointernal/cli/rankings_cmd.gointernal/cli/root.gointernal/cli/search_cmd.gointernal/cli/tags_browse_cmd.gointernal/cli/user_cmd.gointernal/cli/user_cmd_test.goscripts/test-architecture.shscripts/test-documentation.shsdk/browse.gosdk/client.gosdk/client_test.gosdk/entity.gosdk/errors.gosdk/lists.gosdk/magnets.gosdk/movie.gosdk/movie_test.gosdk/rankings.gosdk/search.gosdk/user.go
🛑 Comments failed to post (2)
sdk/client.go (1)
106-117: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
传播所有公开 SDK 方法的
context.Context。这些方法接受
context.Context,但立即使用_ = ctx丢弃它。调用方的取消和 deadline 不会中断请求。WithTimeout的固定客户端超时不能替代每个调用的 context 控制。请在
appapi和 HTTP 请求层添加 context-aware 方法,并使用http.NewRequestWithContext或等效机制传递ctx。
sdk/client.go#L106-L117: 将Login和ResolveUserID的ctx传递到下层。sdk/search.go#L16-L18: 将Search的ctx传递到下层。sdk/browse.go#L14-L34: 将 taxonomy 和Browse方法的ctx传递到下层。sdk/entity.go#L13-L33: 将实体查询和分页聚合的ctx传递到下层。sdk/lists.go#L6-L20: 将列表请求的ctx传递到下层。sdk/rankings.go#L13-L33: 将排行榜请求的ctx传递到下层。sdk/user.go#L6-L38: 将用户读取和状态修改请求的ctx传递到下层。📍 Affects 7 files
sdk/client.go#L106-L117(this comment)sdk/search.go#L16-L18sdk/browse.go#L14-L34sdk/entity.go#L13-L33sdk/lists.go#L6-L20sdk/rankings.go#L13-L33sdk/user.go#L6-L38🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/client.go` around lines 106 - 117, Propagate each public SDK method’s context.Context through the full request stack instead of discarding it. In sdk/client.go lines 106-117, update Login and ResolveUserID; in sdk/search.go lines 16-18, Search; in sdk/browse.go lines 14-34, taxonomy and Browse methods; in sdk/entity.go lines 13-33, entity queries and pagination aggregation; in sdk/lists.go lines 6-20, list requests; in sdk/rankings.go lines 13-33, ranking requests; and in sdk/user.go lines 6-38, user reads and mutations. Add context-aware appapi and HTTP methods, constructing requests with http.NewRequestWithContext or an equivalent mechanism so cancellation and deadlines reach the underlying requests.sdk/movie.go (1)
27-42: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/sh set -eu ast-grep outline sdk/movie.go --items all ast-grep outline internal/javdb/appapi --items all --type function rg -n -C 4 --type go \ 'func \(c \*Client\) (MovieDetail|MovieMagnets|MovieComments|ResolveMovieID|DownloadImage|DownloadHLS|fetchMedia)\b|NewRequestWithContext|\.Do\(' \ sdk internal/javdb/appapiRepository: FlanChanXwO/javdb-cli
Length of output: 9433
🏁 Script executed:
#!/bin/sh set -eu sed -n '1,220p' sdk/movie.go printf '\n--- internal/javdb/appapi/client.go ---\n' sed -n '1,240p' internal/javdb/appapi/client.go printf '\n--- internal/javdb/appapi/movie.go ---\n' sed -n '1,120p' internal/javdb/appapi/movie.go printf '\n--- internal/javdb/appapi/resolve.go ---' sed -n '1,120p' internal/javdb/appapi/resolve.go printf '\n--- internal/javdb/appapi/media.go ---\n' sed -n '1,160p' internal/javdb/appapi/media.go printf '\n--- httpx Client methods ---\n' rg -n -C 3 --type go 'func \(.*httpx\.Client.*\)|type Client|Do\(|Get\(' internal/sdk internal/javdb/protocol internal/javdbRepository: FlanChanXwO/javdb-cli
Length of output: 24816
让
ctx控制网络调用。
sdk/movie.go在第 28、34、41、48 和 160 行丢弃了context.Context,但appapi的请求、图片下载和 HLS 下载路径也没有接收并传递 context。调用方取消 context 或到达 deadline 后,影片详情、磁链、评论、解析、图片下载和 HLS 请求仍会继续执行。将context.Context引入appapi请求、DownloadImage、fetchMedia与 HLS 下载路径并正确传递。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/movie.go` around lines 27 - 42, Propagate the caller’s context instead of discarding it throughout the SDK movie flows: update the appapi request methods used by MovieDetail, MovieMagnets, MovieComments, and the other affected methods, plus DownloadImage, fetchMedia, and the HLS download path, to accept and pass context into their HTTP requests and downloads. Ensure cancellation and deadlines stop all corresponding network operations, and remove the `_ = ctx` placeholders.
Summary
javdb/tosdk/while retainingpackage javdb.Scope and compatibility
github.com/FlanChanXwO/javdb-cli/javdbis replaced bygithub.com/FlanChanXwO/javdb-cli/sdk.javdb; CLI commands, flags, JSON output, configuration, authentication, and release assets are unchanged.Verification
All configured hooks passed: gofmt,
go test ./..., release tooling, release notes, documentation structure, and architecture structure checks.Release note declaration
Checklist
Summary by Sourcery
将公共 Go SDK 门面从顶层的
javdb目录移动到新的sdk目录,同时保持javdb包的 API 不变,并相应更新所有引用。Enhancements:
sdk路径导入公共 SDK,同时继续使用javdb包。sdk位置,并防止使用已废弃的javdb导入路径。Original summary in English
Summary by Sourcery
Move the public Go SDK facade from the top-level javdb directory to a new sdk directory while preserving the javdb package API and update references accordingly.
Enhancements:
Summary by CodeRabbit
新功能
文档