Skip to content

fix: PR #50 review feedback - cold-start XHS routing & verify glob - #60

Closed
aq5115325-cyber wants to merge 2 commits into
XBuilderLAB:mainfrom
aq5115325-cyber:fix/pr50-review-issues
Closed

fix: PR #50 review feedback - cold-start XHS routing & verify glob#60
aq5115325-cyber wants to merge 2 commits into
XBuilderLAB:mainfrom
aq5115325-cyber:fix/pr50-review-issues

Conversation

@aq5115325-cyber

Copy link
Copy Markdown

Addresses both review issues from @Jooonnn on PR #50:

Fix 1: Cold-start XHS routing bug
Added Q1.2 follow-up question for short-text users to specify their target platform (xiaohongshu/weibo-x/other). Starter selection now checks both \enabled_perf_adapters\ (for existing users) and \short_text_platform\ (for cold-start users).

Fix 2: Verify script glob coverage
Shell syntax check find glob now includes \�dapters/script-extraction/*/run.sh\ to cover the whisper adapter.

See individual commits for full changes.

@Jooonnn

Jooonnn commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Thanks for picking up the review items from #50! The intent is exactly right, but this PR can't be merged as-is — a few things to sort out first:

1. skills/cheat-init/SKILL.md is a whole-file rewrite. The diff is a single @@ -1,535 +1,549 @@ hunk (every line deleted and re-added), which almost always means the line endings got converted (CRLF↔LF) or the file was fully re-saved. That hides the actual change and will conflict with anything else touching cheat-init. Could you normalize line endings (keep LF) and re-commit so the diff shows only the real Q1.2 routing change? git config core.autocrlf / a .gitattributes * text=auto eol=lf usually fixes this.

2. tools/verify-package.sh (+198) is duplicated from #50. That file doesn't exist on main yet — it's introduced by #50 (@haoabcde), which is still an open draft. So this PR is effectively re-submitting #50's work plus the fixes, under a different author. That tangles authorship and will collide when #50 lands.

Suggested path: since #50 owns verify-package.sh and the cheat-init routing, the cleanest place for these two fixes is inside #50 itself. Would you be up for sending the cold-start XHS routing fix + the verify-glob fix as a patch/commit to @haoabcde's branch (or coordinating on #50)? If #50 has stalled, an alternative is to rebase this PR onto the latest main and trim it down to just the minimal, reviewable changes (no full-file rewrite, no duplicated file).

Happy to review again once it's down to a clean, minimal diff. 🙏

@Jooonnn

Jooonnn commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

感谢你主动接手 #50 的 review 意见 @aq5115325-cyber —— 方向和判断都是对的。

关闭原因是这两处修复已经被 #50 自己覆盖了,我核对过:

加上这个分支现在跟 main 已经冲突,与其解冲突不如让 #50 单独收口,避免两个 PR 改同一片区域互相打架。

之前提的整文件重写(换行符 CRLF↔LF)问题值得记一下:git config core.autocrlf.gitattributes* text=auto eol=lf 可以避免,以后提 PR 会顺很多。

欢迎继续贡献。

@Jooonnn Jooonnn closed this Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants