Skip to content

Small patch to fix release CI/CD tests #35

Merged
ypriverol merged 3 commits intomainfrom
dev
Apr 6, 2026
Merged

Small patch to fix release CI/CD tests #35
ypriverol merged 3 commits intomainfrom
dev

Conversation

@ypriverol
Copy link
Copy Markdown
Member

@ypriverol ypriverol commented Apr 6, 2026

PR checklist

  • This comment contains a description of changes (with reason).
  • If you've fixed a bug or added code that should be tested, add tests!
  • If you've added a new tool - have you followed the pipeline conventions in the contribution docs
  • If necessary, also make a PR on the bigbio/quantmsdiann branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core pipelines lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Check for unexpected warnings in debug mode (nextflow run . -profile debug,test,docker --outdir <OUTDIR>).
  • Usage Documentation in docs/usage.md is updated.
  • Output Documentation in docs/output.md is updated.
  • CHANGELOG.md is updated.
  • README.md is updated (including new tool citations and authors/contributors).

Summary by CodeRabbit

  • Chores
    • Improved CI reliability: saving PR number now only runs when a PR is present, and PR comment posting is skipped when the PR number is unavailable to avoid workflow failures.
    • Updated lint configuration to exempt the CI linting workflow file from lint-change detection.

ypriverol and others added 2 commits April 6, 2026 09:58
The linting workflow triggers on both pull_request and release events.
On release events, github.event.pull_request.number is empty, causing
PR_number.txt to contain an invalid value and the comment workflow to fail.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 6, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a34f9051-2dd6-4685-92fe-66f350f3371c

📥 Commits

Reviewing files that changed from the base of the PR and between b54dcf3 and 2a88236.

📒 Files selected for processing (1)
  • .nf-core.yml
✅ Files skipped from review due to trivial changes (1)
  • .nf-core.yml

📝 Walkthrough

Walkthrough

Updated GitHub Actions workflows and configuration to avoid failures when no pull request context exists: the linting workflow only writes the PR number when github.event.pull_request.number is present, and the linting_comment workflow sets skip=true and exits successfully when a PR number cannot be determined, skipping comment posting.

Changes

Cohort / File(s) Summary
Linting workflow
.github/workflows/linting.yml
Save PR number step now runs only when always() is true AND github.event.pull_request.number exists, preventing creation of PR_number.txt for non-PR events.
Linting comment workflow
.github/workflows/linting_comment.yml
“Get PR number” step sets skip=true via $GITHUB_OUTPUT and exits 0 when PR number is missing/invalid; “Post PR comment” step guarded by steps.pr_number.outputs.skip != 'true'.
Lint config
.nf-core.yml
Added .github/workflows/linting.yml to lint.files_unchanged list to treat that workflow as exempt from lint-change checks.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • jpfeuffer
  • yueqixuan
  • daichengxin

Poem

"I nibble lines of CI and yarn,
I skip where PRs are not born,
Logs stay tidy, jobs stay bright,
A hopping fix in dev's delight — 🐇"

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Small patch to fix release CI/CD tests' is vague and does not clearly describe the specific changes made (workflow condition updates and configuration changes). Consider using a more specific title that describes the actual changes, such as 'Fix CI/CD workflows: add PR number condition checks and update linting config' to better convey the technical nature of the modifications.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dev

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.

@qodo-code-review
Copy link
Copy Markdown

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

Review Summary by Qodo

Fix linting workflow to skip PR comment on non-PR events

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Fix linting workflow failing on release events due to empty PR number
• Skip PR comment step when no valid PR number is available
• Gracefully handle missing or invalid PR_number.txt instead of failing
Diagram
flowchart LR
  A["linting.yml trigger"] -- "pull_request event" --> B["Save PR number"]
  A -- "release event" --> C["Skip Save PR number"]
  B -- "upload artifact" --> D["linting_comment.yml"]
  C -- "no artifact" --> D
  D -- "PR_number.txt missing" --> E["Skip comment, exit 0"]
  D -- "invalid PR number" --> E
  D -- "valid PR number" --> F["Post PR comment"]
Loading

Grey Divider

File Changes

1. .github/workflows/linting.yml 🐞 Bug fix +1/-1

Conditionally save PR number only on PR events

• Added condition to only save PR_number.txt when github.event.pull_request.number is present
• Prevents writing an empty/invalid value during release events

.github/workflows/linting.yml


2. .github/workflows/linting_comment.yml 🐞 Bug fix +7/-4

Gracefully skip PR comment when no valid PR number

• Changed missing PR_number.txt handling from hard failure to graceful skip
• Changed invalid PR number handling from hard failure to graceful skip
• Added skip=true output flag when PR number is unavailable or invalid
• Added condition on "Post PR comment" step to skip when skip=true

.github/workflows/linting_comment.yml


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review bot commented Apr 6, 2026

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX Issues (0)

Grey Divider


Remediation recommended

1. PR_number.txt omitted from artifact 🐞 Bug ☼ Reliability
Description
In linting.yml, PR_number.txt is only written when github.event.pull_request.number exists, but it
is still listed in the uploaded artifact paths. This means non-PR runs (e.g., release) will upload
artifacts without PR_number.txt, driving the downstream comment workflow into its skip path.
Code

.github/workflows/linting.yml[R68-72]

      - name: Save PR number
-        if: ${{ always() }}
+        if: ${{ always() && github.event.pull_request.number }}
        run: echo ${{ github.event.pull_request.number }} > PR_number.txt

      - name: Upload linting log file artifact
Evidence
The linting workflow runs on both pull_request and release events, but PR_number.txt is only created
when a PR number exists; meanwhile, the artifact upload still includes PR_number.txt in its path
list. This guarantees PR_number.txt is absent on non-PR runs, which is the input the downstream
workflow relies on to decide whether to comment.

.github/workflows/linting.yml[5-9]
.github/workflows/linting.yml[68-80]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`PR_number.txt` is conditionally created but always referenced as an artifact path. On non-PR events this produces an artifact without `PR_number.txt`, forcing downstream logic into a skip path.

### Issue Context
The workflow is triggered by both `pull_request` and `release`, but the PR number only exists for PR events.

### Fix Focus Areas
- .github/workflows/linting.yml[68-80]

### Suggested fix
Choose one:
1) Always create `PR_number.txt` (empty when not a PR), so artifact contents are consistent.
2) Make `PR_number.txt` upload conditional (e.g., separate upload step guarded by the same PR-only condition), while still uploading lint outputs for all events.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Silent lint comment skipping 🐞 Bug ✧ Quality
Description
In linting_comment.yml, missing/invalid PR_number.txt now exits 0 and sets skip=true, so the
workflow succeeds but posts no comment. If PR_number.txt is unexpectedly missing for an actual PR
run (artifact/download issue), this change removes the failure signal and makes the regression hard
to notice.
Code

.github/workflows/linting_comment.yml[R21-38]

        id: pr_number
        run: |
          if [ ! -f linting-logs/PR_number.txt ]; then
-            echo "PR number file not found"
-            exit 1
+            echo "No PR number file found (not a PR event), skipping comment"
+            echo "skip=true" >> $GITHUB_OUTPUT
+            exit 0
          fi
          PR_NUM=$(cat linting-logs/PR_number.txt)
          if ! [[ "$PR_NUM" =~ ^[0-9]+$ ]]; then
-            echo "Invalid PR number: $PR_NUM"
-            exit 1
+            echo "Invalid PR number: $PR_NUM, skipping comment"
+            echo "skip=true" >> $GITHUB_OUTPUT
+            exit 0
          fi
          echo "pr_number=$PR_NUM" >> $GITHUB_OUTPUT

      - name: Post PR comment
+        if: ${{ steps.pr_number.outputs.skip != 'true' }}
        uses: marocchino/sticky-pull-request-comment@773744901bac0e8cbb5a0dc842800d45e9b2b405 # v2
Evidence
The comment workflow now treats missing/invalid PR_number.txt as a success-and-skip condition and
gates the comment step on that output. Because the upstream linting workflow runs on both PR and
release, and the comment workflow does not validate the triggering event type, it cannot distinguish
an expected non-PR run from an unexpected missing PR number on a PR run.

.github/workflows/linting_comment.yml[20-38]
.github/workflows/linting.yml[5-9]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The comment workflow now silently succeeds when `PR_number.txt` is missing/invalid, which can hide real problems on PR runs.

### Issue Context
`linting_comment.yml` is triggered by `workflow_run` and the upstream workflow also runs on `release`, so skipping is needed for non-PR events—but should not hide unexpected failures for PR events.

### Fix Focus Areas
- .github/workflows/linting_comment.yml[20-38]

### Suggested fix
Add an explicit check of the triggering run’s event type (or presence of PRs in the workflow_run payload) and:
- Skip early only when it’s definitively not a PR-triggered run.
- Fail (exit 1) when it is a PR-triggered run but `PR_number.txt` is missing/invalid, so maintainers get a visible signal.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@ypriverol ypriverol changed the title Dev Small patch to fix release CI/CD tests Apr 6, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 6, 2026

nf-core pipelines lint overall result: Passed ✅ ⚠️

Posted for pipeline commit 2a88236

+| ✅ 106 tests passed       |+
#| ❔  19 tests were ignored |#
#| ❔   1 tests had warnings |#
!| ❗   4 tests had warnings |!
Details

❗ Test warnings:

  • files_exist - File not found: conf/igenomes.config
  • files_exist - File not found: conf/igenomes_ignored.config
  • files_exist - File not found: .github/workflows/awstest.yml
  • files_exist - File not found: .github/workflows/awsfulltest.yml

❔ Tests ignored:

❔ Tests fixed:

✅ Tests passed:

Run details

  • nf-core/tools version 3.5.2
  • Run at 2026-04-06 09:03:15

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/linting.yml:
- Line 69: The linting job contains an unnecessary PR-number guard (the line
with the if-condition checking github.event.pull_request.number) which causes
template drift and breaks the nf-core template check; remove that conditional so
the step runs under the workflow's existing guards (or replace it with a simple
always() check) to allow linting_comment.yml to handle missing/invalid PR
numbers and restore template compatibility.
🪄 Autofix (Beta)

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: defaults

Review profile: CHILL

Plan: Pro

Run ID: e43ef97e-ea3d-45c3-9531-0e2a161ba8bc

📥 Commits

Reviewing files that changed from the base of the PR and between 876342a and b54dcf3.

📒 Files selected for processing (2)
  • .github/workflows/linting.yml
  • .github/workflows/linting_comment.yml


- name: Save PR number
if: ${{ always() }}
if: ${{ always() && github.event.pull_request.number }}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Line 69 causes template drift and keeps files_unchanged lint failing

This condition is not needed anymore now that linting_comment.yml safely skips invalid/missing PR numbers, and it appears to be the reason the nf-core template check is failing in this PR.

Suggested template-compatible fix
-      - name: Save PR number
-        if: ${{ always() && github.event.pull_request.number }}
+      - name: Save PR number
+        if: ${{ always() }}
         run: echo ${{ github.event.pull_request.number }} > PR_number.txt

Based on learnings: Run nf-core pipelines lint before merging code to ensure pipeline standards compliance (use --release flag for master branch PRs).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/linting.yml at line 69, The linting job contains an
unnecessary PR-number guard (the line with the if-condition checking
github.event.pull_request.number) which causes template drift and breaks the
nf-core template check; remove that conditional so the step runs under the
workflow's existing guards (or replace it with a simple always() check) to allow
linting_comment.yml to handle missing/invalid PR numbers and restore template
compatibility.

@ypriverol ypriverol merged commit baef81d into main Apr 6, 2026
41 checks passed
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