-
Notifications
You must be signed in to change notification settings - Fork 10
Add YAML linting and Python complexity tooling #757
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
25c0458
86f36bf
858dce0
24e0d97
167a050
34a3426
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| [global] | ||
| require-virtualenv = true |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -83,11 +83,23 @@ jobs: | |||||
| uses: flox/activate-action@v1 | ||||||
| with: | ||||||
| command: mask development markdown all | ||||||
| run_yaml_code_checks: | ||||||
| name: Run YAML code checks | ||||||
| runs-on: ubuntu-latest | ||||||
| steps: | ||||||
| - name: Checkout code | ||||||
| uses: actions/checkout@v4 | ||||||
| - name: Install Flox | ||||||
| uses: flox/install-flox-action@v2 | ||||||
| - name: Run YAML code checks | ||||||
| uses: flox/activate-action@v1 | ||||||
| with: | ||||||
| command: mask development yaml all | ||||||
| upload_test_coverage: | ||||||
| needs: | ||||||
| - run_rust_code_checks | ||||||
| - run_python_code_checks | ||||||
| if: ${{ always() && (needs.run_rust_code_checks.result == 'success' || needs.run_python_code_checks.result == 'success') }} | ||||||
| if: always() && (needs.run_rust_code_checks.result == 'success' && needs.run_python_code_checks.result == 'success') | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Coverage upload condition changed from OR to AND The original condition used
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/run_code_checks.yaml
Line: 102:102
Comment:
**Coverage upload condition changed from OR to AND**
The original condition used `||`, meaning coverage was uploaded if *either* Rust or Python checks succeeded. The new condition uses `&&`, requiring *both* to succeed. This means if one suite has a transient failure, neither suite's coverage gets uploaded — potentially losing valid coverage data from the passing suite. Was this intentional?
```suggestion
if: always() && (needs.run_rust_code_checks.result == 'success' || needs.run_python_code_checks.result == 'success')
```
How can I resolve this? If you propose a fix, please make it concise.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changing Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/run_code_checks.yaml
Line: 102:102
Comment:
Changing `||` to `&&` means coverage only uploads when both Rust and Python checks pass. If one suite has a transient failure, valid coverage data from the passing suite won't be uploaded. This was flagged in previous threads - was this change intentional?
How can I resolve this? If you propose a fix, please make it concise. |
||||||
| name: Upload coverage to Coveralls | ||||||
| runs-on: ubuntu-latest | ||||||
| steps: | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| --- | ||
| extends: default | ||
|
|
||
| ignore: | | ||
| .venv/ | ||
| target/ | ||
| .flox/ | ||
|
|
||
| rules: | ||
| line-length: | ||
| max: 120 | ||
| truthy: | ||
| allowed-values: ['true', 'false', 'on'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The conditional logic has been changed from OR to AND. Previously, coverage would be uploaded if EITHER Rust OR Python checks succeeded. Now it requires BOTH to succeed. This is a significant behavioral change that means if either language's tests fail, no coverage will be uploaded at all. This may be intentional to ensure complete coverage data, but it's a breaking change in the workflow behavior that should be highlighted.