Skip to content

Conversation

@CoMPaTech
Copy link
Member

@CoMPaTech CoMPaTech commented Sep 24, 2025

Summary by CodeRabbit

  • Tests

    • Streamlined the test bootstrap by executing the bootstrap script directly, improving reliability and consistency of the test setup in CI.
  • Chores

    • Minor script cleanup and indentation adjustments.
    • No impact on user-facing functionality.

@CoMPaTech CoMPaTech requested a review from a team as a code owner September 24, 2025 10:01
@CoMPaTech CoMPaTech added bug Something isn't working core-dev-compat Home Assistant Core Release Candidate or Beta compatibility labels Sep 24, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 24, 2025

Walkthrough

Replaced a piped/bootstrap invocation in scripts/core-testing.sh with a direct execution sh "${coredir}/script/bootstrap" and enabled shell tracing (set -ex); surrounding indentation adjusted. No changes to public interfaces. (≤50 words)

Changes

Cohort / File(s) Summary of Changes
Core testing script
scripts/core-testing.sh
Enabled set -ex; replaced the previous pipeline-based bootstrap call with direct sh "${coredir}/script/bootstrap"; minor indentation/formatting tweaks.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant User as Runner
    participant Script as scripts/core-testing.sh
    participant Bootstrap as script/bootstrap
    participant Pip as pip/install steps

    User->>Script: execute core-testing.sh
    Note right of Script: set -ex (trace & exit-on-error)
    Script->>Bootstrap: sh "${coredir}/script/bootstrap"
    Bootstrap->>Pip: prepare environment / packages
    Pip-->>Bootstrap: install complete
    Bootstrap-->>Script: return
    Script->>Script: proceed with remaining install steps
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I hopped through logs with tiny feet,
Replaced a pipe with something neat.
"sh" I said, and bootstrap ran—
No more grep to twist the plan.
Thump-thump! — the tests now hum, complete. 🐇✨

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 "Fix core bootstrapping" accurately and concisely reflects the primary change shown in the diff, which updates scripts/core-testing.sh to run the bootstrap script directly rather than via the previous pipeline; it is focused, clear, and immediately conveys the intent to fix bootstrapping.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch uvfix

📜 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 3ada475 and 2f0defa.

📒 Files selected for processing (1)
  • scripts/core-testing.sh (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • scripts/core-testing.sh
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Setup for HA-core (dev)
  • GitHub Check: Setup for HA-core (release/master)
  • GitHub Check: Prepare and validate pre-commit

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.

@CoMPaTech CoMPaTech self-assigned this Sep 24, 2025
Copy link
Contributor

@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)
scripts/core-testing.sh (1)

2-2: Avoid global set -x; add pipefail and gate tracing to prevent secret leakage

Global tracing can leak tokens/credentials in CI logs and adds noise. Prefer failing fast with pipefail and enable tracing only when needed (e.g., local DEBUG or non‑CI).

Apply:

-set -ex
+set -Eeuo pipefail
+# Trace only when debugging locally to avoid leaking secrets in CI logs
+[ -z "${GITHUB_ACTIONS:-}" ] && [ -n "${DEBUG:-}" ] && set -x
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f0c1541 and 3ada475.

📒 Files selected for processing (1)
  • scripts/core-testing.sh (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CoMPaTech
PR: plugwise/plugwise-beta#789
File: .pre-commit-config.yaml:3-3
Timestamp: 2024-12-08T11:23:47.692Z
Learning: CoMPaTech considers Python 3.13 stable and prefers to use it in the project.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Prepare
  • GitHub Check: Prepare
  • GitHub Check: Prepare
🔇 Additional comments (1)
scripts/core-testing.sh (1)

184-184: Do not change — script/bootstrap uses /bin/sh

Verified remote script/bootstrap has a shebang of "#!/bin/sh" (so invoking with sh matches the script); switching to bash is unnecessary.

Likely an incorrect or invalid review comment.

@sonarqubecloud
Copy link

@CoMPaTech
Copy link
Member Author

Fixes #928

@CoMPaTech CoMPaTech linked an issue Sep 24, 2025 that may be closed by this pull request
@CoMPaTech CoMPaTech merged commit 2942752 into main Sep 24, 2025
13 checks passed
@CoMPaTech CoMPaTech deleted the uvfix branch September 24, 2025 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working core-dev-compat Home Assistant Core Release Candidate or Beta compatibility

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Next Home Assistant version incompatibility

1 participant