ci: add Fedora 44 and Ubuntu 26.04 to GitHub automation#2600
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds CI support for Ubuntu 26.04 (container and direct) and Fedora 44: new platform configs, workflow matrix and dispatch updates, Ubuntu 26.04 package lists and Packagecloud mapping, QA host shortname mapping, a Pylint config tweak, and a small CLI inject handling change. ChangesUbuntu 26.04 and Fedora 44 CI platform expansion
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
build/ci/platforms/fedora44-container.yml (1)
45-45:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd missing trailing newline.
This file does not end with a trailing newline character. As per coding guidelines, all text and source files must end with a single trailing newline character.
📝 Proposed fix
Ensure the file ends with a newline character after line 44.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@build/ci/platforms/fedora44-container.yml` at line 45, The file fedora44-container.yml is missing a final newline; open that YAML and ensure the file ends with a single trailing newline character (i.e., add a newline after the current last line so the file terminates with '\n').build/ci/platforms/ubuntu2604.yml (1)
34-34:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd missing trailing newline.
This file does not end with a trailing newline character. As per coding guidelines, all text and source files must end with a single trailing newline character.
📝 Proposed fix
Ensure the file ends with a newline character after line 33.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@build/ci/platforms/ubuntu2604.yml` at line 34, The file ubuntu2604.yml is missing a trailing newline; open ubuntu2604.yml (the CI config shown) and add a single newline character at the end of the file so the file ends with exactly one trailing newline (remove any extra empty lines at the end if present), then save and commit the change.build/ci/platforms/ubuntu2604-container.yml (1)
54-54:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd missing trailing newline.
This file does not end with a trailing newline character. As per coding guidelines, all text and source files must end with a single trailing newline character.
📝 Proposed fix
Ensure the file ends with a newline character after line 53.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@build/ci/platforms/ubuntu2604-container.yml` at line 54, Add a single trailing newline character to the end of the YAML file so it ends with exactly one newline (i.e., ensure the file terminates with a newline character and no extra blank lines); update the file content to insert the missing newline at EOF.qa/admin/qa-summary (1)
797-798:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the extra trailing newline.
The file ends with two empty lines (797 and 798), resulting in two trailing newlines. As per coding guidelines, all text and source files should end with a single trailing newline character. Please remove line 798.
Proposed fix
Delete the extra blank line at the end of the file so that only one trailing newline remains after line 796's closing
fi.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@qa/admin/qa-summary` around lines 797 - 798, Remove the extra trailing blank line at the end of the file so it ends with a single newline after the closing "fi" (delete the empty line following the final fi to leave only one trailing newline).qa/admin/package-lists/Ubuntu+26.04+any (1)
1-148:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix Ubuntu 26.04 package list entries for ICU/Perl
qa/admin/package-lists/Ubuntu+26.04+anyincludeslibperl5.40t64, but Ubuntu 26.04 provideslibperl5.40(not64)—this entry will likely fail to install.libicu76isn’t available in Ubuntu 26.04; uselibicu78instead.perl-modules-5.40is available in Ubuntu 26.04.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@qa/admin/package-lists/Ubuntu`+26.04+any around lines 1 - 148, The package list contains incorrect ICU/Perl package names; replace the entry libperl5.40t64 with libperl5.40 and replace libicu76 with libicu78, and ensure perl-modules-5.40 is present (keep or add it) so the Ubuntu 26.04 package names match the distro; update the lines referencing libperl5.40t64 and libicu76 accordingly in the package list.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@build/ci/platforms/fedora44-container.yml`:
- Line 45: The file fedora44-container.yml is missing a final newline; open that
YAML and ensure the file ends with a single trailing newline character (i.e.,
add a newline after the current last line so the file terminates with '\n').
In `@build/ci/platforms/ubuntu2604-container.yml`:
- Line 54: Add a single trailing newline character to the end of the YAML file
so it ends with exactly one newline (i.e., ensure the file terminates with a
newline character and no extra blank lines); update the file content to insert
the missing newline at EOF.
In `@build/ci/platforms/ubuntu2604.yml`:
- Line 34: The file ubuntu2604.yml is missing a trailing newline; open
ubuntu2604.yml (the CI config shown) and add a single newline character at the
end of the file so the file ends with exactly one trailing newline (remove any
extra empty lines at the end if present), then save and commit the change.
In `@qa/admin/package-lists/Ubuntu`+26.04+any:
- Around line 1-148: The package list contains incorrect ICU/Perl package names;
replace the entry libperl5.40t64 with libperl5.40 and replace libicu76 with
libicu78, and ensure perl-modules-5.40 is present (keep or add it) so the Ubuntu
26.04 package names match the distro; update the lines referencing
libperl5.40t64 and libicu76 accordingly in the package list.
In `@qa/admin/qa-summary`:
- Around line 797-798: Remove the extra trailing blank line at the end of the
file so it ends with a single newline after the closing "fi" (delete the empty
line following the final fi to leave only one trailing newline).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 8b50abbb-3664-41e3-9d83-df3b581c9d81
📒 Files selected for processing (11)
.github/workflows/ci.yml.github/workflows/qa.yml.github/workflows/release.ymlbuild/ci/README.mdbuild/ci/packagecloud.shbuild/ci/platforms/fedora44-container.ymlbuild/ci/platforms/ubuntu2604-container.ymlbuild/ci/platforms/ubuntu2604.ymlqa/admin/package-lists/Ubuntu+26.04+anyqa/admin/package-lists/Ubuntu+26.04+x86_64qa/admin/qa-summary
@coderabbitai The proposed fix for YAML files is IMO wrong. YAML files should not end with a new line. At least |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check for yamllint configuration files
fd -e yml -e yaml '.yamllint' .
fd '.yamllint' .
find . -name '.yamllint*' -o -name 'yamllint*' 2>/dev/null | head -20
# Check if existing (older) platform YAML files end with a newline
echo "=== Checking existing platform YAML files for trailing newlines ==="
for f in build/ci/platforms/fedora42-container.yml build/ci/platforms/fedora43-container.yml build/ci/platforms/ubuntu2404-container.yml build/ci/platforms/ubuntu2204-container.yml; do
if [ -f "$f" ]; then
last=$(tail -c 1 "$f" | xxd)
echo "$f: last byte: $last"
fi
done
# Also check the new files
for f in build/ci/platforms/fedora44-container.yml build/ci/platforms/ubuntu2604.yml build/ci/platforms/ubuntu2604-container.yml; do
if [ -f "$f" ]; then
last=$(tail -c 1 "$f" | xxd)
echo "$f: last byte: $last"
fi
doneLength of output: 1653 🌐 Web query:
Length of output: 10110
According to the yamllint
This rule is enabled by default in yamllint's default configuration: rules:
new-line-at-end-of-file: enableSo yamllint will FAIL a YAML file that does not end with a trailing newline — and PASS one that does. My original comments flagging the missing trailing newlines in the new platform files were correct. The confusion may be with a different concern — yamllint's |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
build/ci/platforms/ubuntu2604-container.yml (1)
1-53:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a single trailing newline to comply with coding standards.
The file does not end with a trailing newline character. As per coding guidelines, all text and source files must end with a single trailing newline character.
📝 Proposed fix
Add a single newline character (
\n) after line 53 to ensure the file ends with exactly one trailing newline.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@build/ci/platforms/ubuntu2604-container.yml` around lines 1 - 53, The file ends without a trailing newline; add a single newline character at the end of the file so it terminates with exactly one trailing newline (edit the end of build/ci/platforms/ubuntu2604-container.yml after the final "copy_test_artifacts" task block to append one '\n').
🧹 Nitpick comments (1)
src/pmdas/lmsensors/pmdalmsensors.python (1)
42-43: ⚡ Quick winAdd error handling for file opening.
The migration from
argparse.FileTypedefers file validation from parse time to runtime. Ifargs.injectpoints to a missing or unreadable file, users will see a generic Python exception instead of a helpful error message.🛡️ Proposed error handling
if args.inject: # we will inject sensor data from a file - with open(args.inject, encoding='utf-8') as f: - output = f.read() + try: + with open(args.inject, encoding='utf-8') as f: + output = f.read() + except FileNotFoundError: + print(f"Error: Inject file not found: {args.inject}") + sys.exit(1) + except PermissionError: + print(f"Error: Cannot read inject file: {args.inject}") + sys.exit(1)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pmdas/lmsensors/pmdalmsensors.python` around lines 42 - 43, Wrap the file read around a try/except to catch OSError (e.g., FileNotFoundError/PermissionError) when opening args.inject, print a clear error to stderr that includes args.inject and the exception message, and exit with a non‑zero status; update the block that currently does "with open(args.inject, encoding='utf-8') as f: output = f.read()" to handle errors and import sys if not already present.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@build/ci/platforms/ubuntu2604-container.yml`:
- Around line 1-53: The file ends without a trailing newline; add a single
newline character at the end of the file so it terminates with exactly one
trailing newline (edit the end of build/ci/platforms/ubuntu2604-container.yml
after the final "copy_test_artifacts" task block to append one '\n').
---
Nitpick comments:
In `@src/pmdas/lmsensors/pmdalmsensors.python`:
- Around line 42-43: Wrap the file read around a try/except to catch OSError
(e.g., FileNotFoundError/PermissionError) when opening args.inject, print a
clear error to stderr that includes args.inject and the exception message, and
exit with a non‑zero status; update the block that currently does "with
open(args.inject, encoding='utf-8') as f: output = f.read()" to handle errors
and import sys if not already present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 567fc805-35ed-41cc-af19-75f21137b15e
📒 Files selected for processing (3)
.pylintrcbuild/ci/platforms/ubuntu2604-container.ymlsrc/pmdas/lmsensors/pmdalmsensors.python
💤 Files with no reviewable changes (1)
- .pylintrc
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pmdas/lmsensors/pmdalmsensors.python`:
- Around line 42-47: The inject branch currently reads args.inject into output
but never populates sensorvalues, so fix it by parsing the injected content into
the same normalized structure used by the live sensors -j path and then running
the same sensor population logic: after reading output in the try block, parse
output (e.g., json.loads or the same parser used in the else branch) to produce
the same data structure and assign/populate sensorvalues exactly as done in the
else path (reuse the population loop/function that transforms sensors -j output
into sensorvalues) so injected metrics follow identical normalization and
downstream handling.
🪄 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: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: bd36be58-7a04-4595-944d-c36674497536
📒 Files selected for processing (1)
src/pmdas/lmsensors/pmdalmsensors.python
Add platform definitions, QA package lists, and workflow matrix entries for fedora44-container, ubuntu2604-container, and ubuntu2604. Map ubuntu2604 releases to the resolute codename for Packagecloud uploads. Drop obsolete suggestion-mode from .pylintrc for pylint 4.x. Stop using deprecated argparse.FileType in the lmsensors PMDA. Use /usr/lib/systemd/systemd as PID 1 on Ubuntu 26.04 where /usr/bin/systemd is no longer shipped. Fix Ubuntu 26.04 package list Perl and ICU names. Report OSError when the lmsensors inject file cannot be read. Co-authored-by: Cursor <cursoragent@cursor.com>
1 participant
Add platform definitions, QA package lists, and workflow matrix entries for fedora44-container, ubuntu2604-container, and ubuntu2604. Map ubuntu2604 releases to the resolute codename for Packagecloud uploads.