Skip to content

ci: tolerate CRLF line endings in pr-issue-check#329

Closed
along-2017 wants to merge 1 commit into
mainfrom
fix/pr-issue-check-crlf
Closed

ci: tolerate CRLF line endings in pr-issue-check#329
along-2017 wants to merge 1 commit into
mainfrom
fix/pr-issue-check-crlf

Conversation

@along-2017

@along-2017 along-2017 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

TL;DR

Fixes a false negative in the pr-issue-check gate: a PR with a valid ## Issues reference fails when its body uses CRLF line endings. The section detector required a bare LF after the ## Issues heading, but GitHub stores PR bodies with CRLF (web UI edits and bot edits such as automated summaries), so the \r made the regex miss the section and report it missing. The fix normalizes the body to LF before matching.

Additional Details

  • Only newline handling changes. The reference, NO-REF, disallowed-tracker, and disallowed-URL checks are untouched.
  • Observed symptom: a correct Closes #NNN still fails with PR body is missing a visible ## Issues section after a bot rewrites the description with CRLF.

For the Reviewer

  • One-line normalization of the body before the ## Issues regex in ci/check-pr-issue.sh.

For QA

  • Ran ci/check-pr-issue.sh against CRLF and LF bodies: CRLF with Closes #NNN now passes and LF still passes, while the negative cases (no ## Issues section, section with no reference, NO-REF, and a disallowed Jira key) all still behave as before.

Issues

NO-REF

Checklist

  • I am familiar with the Contributing Guidelines.
  • I have signed off my commits for Developer Certificate of Origin (DCO) compliance.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

Summary by CodeRabbit

  • Bug Fixes
    • Improved pull request issue-section detection for content using Windows-style line endings.
    • Ensured line-ending variations are handled consistently during validation.

The ## Issues section detector matched `^##[ \t]+Issues[ \t]*\n`, which
requires a bare LF after the heading. GitHub stores PR bodies with CRLF
line endings (web UI edits and bot edits such as automated summaries), so
on those bodies the `\r` before the `\n` made the regex miss the section
and the check failed with "PR body is missing a visible ## Issues section"
even when a valid `Closes #NNN` reference was present.

Normalize the body's newlines to LF once before matching. This fixes the
false negative without loosening any of the reference, NO-REF, disallowed
tracker, or disallowed URL checks.

Signed-off-by: along <along@nvidia.com>
@along-2017
along-2017 requested a review from a team as a code owner July 21, 2026 22:17
@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 229f0d6c-6c1e-493c-9ebb-1ab8b56af596

📥 Commits

Reviewing files that changed from the base of the PR and between d6c2206 and b725e7a.

📒 Files selected for processing (1)
  • ci/check-pr-issue.sh

📝 Walkthrough

Walkthrough

The PR issue checker now normalizes CRLF and CR line endings to LF before matching the visible ## Issues section.

Changes

Issue checker

Layer / File(s) Summary
Normalize PR body before matching
ci/check-pr-issue.sh
Converts CRLF and CR line endings to LF before applying the Issues section regular expression.

Estimated code review effort: 2 (Simple) | ~5 minutes

Suggested reviewers: kristinapathak

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: making the PR issue check tolerate CRLF line endings.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/pr-issue-check-crlf

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

@along-2017 along-2017 self-assigned this Jul 21, 2026
@balajinvda

Copy link
Copy Markdown
Contributor

I think this check was removed from main and we are now using coderabbit for this.

@sbaum1994

Copy link
Copy Markdown
Contributor

Closing per balaji's comment

@sbaum1994 sbaum1994 closed this Jul 23, 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.

3 participants