Skip to content

Conversation

@hu-qi
Copy link
Collaborator

@hu-qi hu-qi commented Dec 5, 2025

Summary by CodeRabbit

  • Style
    • Notification messages now strip code blocks and inline code and display full link text (URLs) for commits, issues, pull requests, discussions, comments, releases, and related links.
  • Chores
    • CI workflows: checkout action versions updated across workflows; event serialization adjusted to emit YAML as a literal string.
    • Build: TypeScript compilation now includes all source files under the src directory.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 5, 2025

Walkthrough

Push-event and other link rendering now use raw URL text instead of custom labels; sanitizeMarkdown reverted to remove code blocks and inline code; workflows updated to actions/checkout@v6; TypeScript config now includes src/**/* in compilation.

Changes

Cohort / File(s) Summary
Push & message transform
/.github/scripts/transform-message-for-card.ts
Re-enabled code-block and inline-code removal in sanitizeMarkdown; removed generation of shortened commitId and replaced custom link labels (e.g., #<number>, 查看评论, short commit ids) so createContentItem calls now use the default URL text.
CI workflows
/.github/workflows/Lark-notification.yml, /.github/workflows/test.yml
Updated actions/checkout usage to @v6. In Lark-notification, the YAML serialization echo was changed to quote the variable (writes literal $YAML). No other steps altered.
TypeScript project includes
/tsconfig.json
Added "include": ["src/**/*"] to the TS project includes; no other compiler options changed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Inspect transform-message-for-card.ts for all event branches to confirm link text fallbacks and that markdown sanitization behaves as expected.
  • Check Lark-notification YAML quoting change doesn't break downstream consumers and verify actions/checkout@v6 compatibility.
  • Validate tsconfig.json inclusion doesn't inadvertently include unwanted files.

Possibly related PRs

Poem

🐇 I nibbled bytes and hopped the stream,
URLs now shine where short tags had been.
Workflows leapt to v6, so spry —
Src files found the open sky. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes a specific change (removing short commit ID from display), but this is only one aspect of a broader refactoring that also changes how link text is displayed across multiple link types (issues, PRs, discussions, etc.) and updates GitHub Actions versions.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6b9dfbd and f0eb259.

📒 Files selected for processing (2)
  • .github/scripts/transform-message-for-card.ts (9 hunks)
  • .github/workflows/Lark-notification.yml (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/Lark-notification.yml
🔇 Additional comments (4)
.github/scripts/transform-message-for-card.ts (4)

61-83: LGTM on sanitizeMarkdown re-enablement.

The code block and inline code removal patterns are correctly implemented. The non-greedy quantifier *? in the code block regex prevents over-matching across multiple blocks.


106-119: LGTM on commit link simplification.

The change removes the short commit ID in favor of using the full URL as link text, which aligns with the PR objective. The fallback URL construction for branch-only events (no head commit) remains intact.


125-213: Consistent link format applied across event handlers.

All handlers now use the URL as the link text, maintaining consistency with the push event changes.


215-228: Potential inconsistency: line 223 still uses custom link text.

Line 221 was updated to use URL-only link text, but line 223 still uses #${pull_request!.number} as custom text. If the intent is to standardize on URL-only link text across all handlers, this should also be updated:

-          createContentItem('PR:', createLink(pull_request!.html_url, `#${pull_request!.number}`)),
+          createContentItem('PR:', createLink(pull_request!.html_url)),

If keeping #${number} here is intentional (e.g., to differentiate the comment link from the PR link within the same card), please disregard this comment.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a8ce930 and 82af247.

📒 Files selected for processing (2)
  • .github/scripts/transform-message-for-card.ts (2 hunks)
  • .github/workflows/Lark-notification.yml (1 hunks)
🔇 Additional comments (3)
.github/scripts/transform-message-for-card.ts (3)

64-64: Code block sanitization looks good.

The regex correctly identifies and replaces Markdown code blocks with a localized placeholder. The non-greedy *? quantifier ensures proper matching of multiple code blocks.


66-66: Inline code sanitization is appropriate.

The regex strips backticks from inline code while preserving the content. Note that it won't match empty inline code (e.g., ` `) or handle escaped backticks, but these edge cases are rare in typical commit messages and descriptions.


115-115: Commit link simplified to use full URL.

This change aligns with the PR objective by removing the shortened commit ID. The link text now displays the full commit URL, which is more verbose but self-contained. The implementation correctly leverages the default parameter of createLink.

Copy link

@coderabbitai coderabbitai bot left a 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 (1)
.github/scripts/transform-message-for-card.ts (1)

221-223: Minor inconsistency in link text format.

Within the same handler, line 221 now uses the URL as link text, while line 223 retains the custom #${pull_request!.number} format. If the goal is consistency, consider aligning these; otherwise, this mixed approach is fine if the distinction is intentional (comment link vs. PR reference).

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9ac5bb9 and 6b9dfbd.

📒 Files selected for processing (1)
  • .github/scripts/transform-message-for-card.ts (8 hunks)
🔇 Additional comments (2)
.github/scripts/transform-message-for-card.ts (2)

115-115: LGTM - aligns with PR objective.

The commit link now displays the URL as the link text instead of the short commit ID, as intended by this refactor.


131-131: Verify: Changes extend beyond stated PR objective.

The PR title indicates "remove short commit ID from commit link display," but these changes also affect link text for issues, pull requests, discussions, comments, and releases. The links will now display full URLs (e.g., https://github.com/org/repo/issues/123) instead of potentially shorter identifiers.

Please confirm this broader scope was intentional. If only the commit link was meant to change, these lines should retain their original link text arguments.

Also applies to: 148-148, 165-165, 179-179, 193-193, 207-207, 221-221

@hu-qi hu-qi closed this Dec 6, 2025
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.

1 participant