-
Notifications
You must be signed in to change notification settings - Fork 2
refactor: remove short commit ID from commit link display. #6
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
Conversation
…ngrade actions/checkout to v4.
…erialization and proper output quoting.
WalkthroughStrips fenced code blocks and inline code during markdown sanitization; stops deriving/passing commitId for push events; standardizes link creation to URL-only anchors; updates GitHub Actions checkout to v6, adds FEISHU_BOT_WEBHOOK_URL env and heredoc JSON handling; adds tsconfig "include": ["src/**/*"]. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/Lark-notification.yml (1)
35-48: Event serialization and GITHUB_OUTPUT handling look correctThe heredoc into
deno runwithtoJSON(github)and capturing it intoYAML(actually JSON) is well-quoted, and the{ echo 'content<<EOF'; echo "$YAML"; echo 'EOF'; } >> "$GITHUB_OUTPUT"pattern correctly emits a multi-line output while protecting$GITHUB_OUTPUTwith quotes. This should be safe even for complex payloads.If you want to be extra defensive, consider using a less common delimiter than
EOFto further reduce the chance of collisions with content lines..github/scripts/transform-message-for-card.ts (2)
61-83: Markdown sanitization for code blocks/inline code is reasonableEnabling
.replace(/```[\s\S]*?```/g, '[代码块]')and the inline-code unwrapping keeps messages compact while preserving the code indication. The order of replacements is safe, and the transformations remain compatible with the later truncation and other markdown cleanups.If you start seeing very large payloads dominated by code, you might consider truncating before running the expensive
[\s\S]*?regex or replacing code blocks with a shorter marker to reduce processing overhead.
96-229: Link text standardization to URL-only is consistent with the PR goalFor push, issues, PRs, discussions, comments, and releases, the “链接:” fields now use
createLink(<url>)so the visible text is the URL itself, which removes short IDs/custom labels as described and keeps behavior consistent across event types. The remaining customPR:link text (#${pull_request!.number}) is still clear and seems intentional.If you later want complete uniformity, you could also switch the
PR:field to URL-only text (or conversely keep human-friendly labels everywhere and drop only the short commit ID), but that’s stylistic rather than functional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/scripts/transform-message-for-card.ts(9 hunks).github/workflows/Lark-notification.yml(1 hunks).github/workflows/test.yml(1 hunks)tsconfig.json(1 hunks)
🔇 Additional comments (3)
tsconfig.json (1)
11-11: Explicit include matches rootDir and is safeAdding
"include": ["src/**/*"]is consistent withrootDir: "./src"and keeps tests excluded via the existingexclude. This should make the compile scope more explicit without side effects, assuming all runtime sources live undersrc/..github/workflows/test.yml (1)
15-15: Checkout upgrade to v6 looks fine; just confirm compatibilityUsing
actions/checkout@v6in both build and test jobs is a good modernization step and should behave the same for this basic usage, but please confirm in CI that there are no surprises from the major-version bump (e.g., changes to default options).Also applies to: 23-23
.github/workflows/Lark-notification.yml (1)
20-33: Checkout bump and Feishu URL guard improve robustnessUpgrading to
actions/checkout@v6and adding an explicitCheck for Feishu URLstep (failing when the secret is unset/empty) both make this workflow more robust and fail-fast instead of sending malformed requests.
…nition at the job level.
…iate exit on missing secret.
test
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.