Skip to content

feat: implement issue #36 — SonarCloud: shell script hygiene - #37

Merged
don-petry merged 2 commits into
mainfrom
dev-lead/issue-36-20260707-1917
Jul 13, 2026
Merged

don-petry merged 2 commits into
mainfrom
dev-lead/issue-36-20260707-1917

Conversation

@don-petry

@don-petry don-petry commented Jul 7, 2026 •

Copy link
Copy Markdown
Contributor

Closes #36

Implemented by dev-lead agent. Please review.

Summary by CodeRabbit

  • Bug Fixes
    • Improved cron backfill script reliability by using the repository’s virtual environment Python when available, with an automatic fallback to python3 if needed.
    • Kept run start/end timestamps in the same UTC ISO-8601 format while simplifying the timestamp helper implementation.

@don-petry
don-petry requested a review from a team as a code owner July 7, 2026 19:18
Copilot AI review requested due to automatic review settings July 7, 2026 19:18
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Jul 7, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c0b10ce7-e61a-4d46-a4ed-1c50d9504cbd

📥 Commits

Reviewing files that changed from the base of the PR and between 94fd8a9 and cda6cab.

📒 Files selected for processing (1)
  • scripts/cron_backfill.sh

📝 Walkthrough

Walkthrough

Modified scripts/cron_backfill.sh to add an executable check for the venv Python path with fallback to python3, and reformatted the ts() helper function from a single-line to a multi-line definition while preserving the same UTC ISO-8601 output format.

Changes

Cron Backfill Script Hygiene

Layer / File(s) Summary
Python interpreter selection and timestamp helper
scripts/cron_backfill.sh
Adds explicit executable check for venv Python path with fallback to python3, and rewrites the ts() helper as a multi-line function returning the same UTC timestamp format.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Related issues: #36

🚥 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 references issue #36 and the shell script hygiene work in scripts/cron_backfill.sh.
Linked Issues check ✅ Passed The summary indicates both SonarCloud findings in scripts/cron_backfill.sh were addressed without changing behavior.
Out of Scope Changes check ✅ Passed All changes stay within scripts/cron_backfill.sh and align with the SonarCloud shell hygiene fixes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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 dev-lead/issue-36-20260707-1917

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.

@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — waiting on PR blockers (intent: review-changes)

PR: #37
No changes were committed, but the PR still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews). The retry cron will re-attempt automatically. Next attempt after: 2026-07-07T19:49:43Z

@don-petry

Copy link
Copy Markdown
Contributor Author

Note

@don-petry I reviewed this PR and no code changes were needed, but it still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews), so I cannot mark it done yet. I'll re-check automatically.
Next attempt after: 2026-07-07T19:49:43Z

@don-petry
don-petry enabled auto-merge (squash) July 7, 2026 19:19

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Addresses SonarCloud shell-script hygiene findings for the cron backfill script (issue #36) without changing intended runtime behavior, by applying safer Bash conditionals and satisfying a function-return rule.

Changes:

  • Replaced POSIX [ test with Bash [[ for executable check on the venv Python path.
  • Reformatted ts() into a multi-line function and added an explicit return to satisfy SonarCloud’s function-return requirement.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@don-petry
don-petry disabled auto-merge July 7, 2026 19:21
@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — fix-reviews (no-changes)

Agent reasoning
Addressed 0 threads:
(no open threads)
Test verification: skipped — no code changes were made
Files changed: none
```
The PR is clean: CodeRabbit approved, all CI checks pass, and no review threads are open.

@donpetry-bot donpetry-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Automated review — APPROVED ✓

Risk: LOW
Reviewed commit: cda6cabc3be0f5ddbe421db2ccc36b11979da966
Review mode: triage-approved (single reviewer)

Summary

Trivial shell-hygiene PR (+5/−2, one file) fixing both SonarCloud findings from issue #36 in scripts/cron_backfill.sh: replaces POSIX '[' with bash '[[' for the venv-python executable check (S7688), and adds an explicit bare 'return' at the end of the ts() helper (S7682). No behavior change: the script already requires bash (uses BASH_SOURCE), and a bare 'return' preserves the exit status of the preceding 'date' command.

Linked issue analysis

Closes #36 (SonarCloud: shell script hygiene, priority:major). The issue lists exactly two findings, both in scripts/cron_backfill.sh: shelldre:S7688 (use '[[' instead of '[') and shelldre:S7682 (add explicit return at end of function). Both are substantively fixed by this diff with real code changes (no NOSONAR suppressions). SonarCloud quality gate on the PR passed with 0 new issues, consistent with the acceptance criteria.

Findings

No blocking findings.

  • Secret scan (MCP): run_secret_scanning tool not available in this environment — skipped; diff contains no credential-like content, and the gitleaks/CI checks remain authoritative.
  • Nit (non-blocking): the bare 'return' at the end of ts() is stylistically unusual but is precisely what SonarCloud rule S7682 asks for, and it is behavior-preserving.
  • Triage confirmation: triage-approved assessment verified — change is confined to one script, matches the linked issue scope exactly, and introduces no new logic paths.

CI status

All checks green: build-and-test ✓, CodeQL (actions + python) ✓, SonarCloud Code Analysis / Quality Gate ✓ (0 new issues, 0 hotspots), AgentShield ✓, dependency-audit / pip-audit ✓. Remaining checks SKIPPED are conditional jobs (dependabot-automerge, npm/pnpm/cargo/go audits, mention handlers). CodeRabbit approved; Copilot reviewer left an overview comment with no actionable items; no unresolved review threads. mergeStateStatus is BLOCKED only pending the required review this agent provides.


Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.

@don-petry
don-petry disabled auto-merge July 8, 2026 14:44
@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

@don-petry
don-petry disabled auto-merge July 13, 2026 23:47
@sonarqubecloud

Copy link
Copy Markdown

@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

@don-petry
don-petry merged commit fb0fbd2 into main Jul 13, 2026
18 checks passed
@don-petry
don-petry deleted the dev-lead/issue-36-20260707-1917 branch July 13, 2026 23:47
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.

SonarCloud: shell script hygiene

3 participants