Repository navigation
aleph: handle missing or null version field - #1178
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes.
 |
| Layer / File(s) | Summary |
|---|---|
Optional version parsing src/aleph.rs |
Aleph.version changes to Option<String>. Parsing tests cover present versions, the legacy build alias, and missing or null versions. |
Consumers of optional versions src/bootupd.rs, src/component.rs |
Status returns None when Aleph metadata or its version is absent. Adoption state records "unknown" when the version is absent. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix
Suggested reviewers: jbtrystram, dustymabe
Merge Risk: 
df13e
-
No merge-blocking issue is identified in the supplied evidence.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within survi… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title "aleph: handle missing or null version field" describes the main change: making the Aleph version field optional so that missing or null values are accepted. It uses the subsystem prefix, lo… Description check The description states that bootc's Aleph implementation defines the version field as Option, so this change does the same. It explains that the change handles the field being unset or set to … Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Commit Message Convention The PR contains one non-merge commit. Its subject is aleph: handle missing or null version field, which uses a valid component prefix, starts the description with the lowercase imperativehandle, …Full details: Title check
-
Explanation
The title "aleph: handle missing or null version field" describes the main change: making the Aleph version field optional so that missing or null values are accepted. It uses the subsystem prefix, lowercase text after the colon, and imperative mood, and has no trailing period, so it meets the stated format. The title does not fully match the example subsystem style (e.g., src/aleph.rs), but 'aleph:' is a clear subsystem name.
Full details: Description check
-
Explanation
The description states that bootc's Aleph implementation defines the version field as Option<String>, so this change does the same. It explains that the change handles the field being unset or set to null, as seen in issue
#1166, and calls it the reader-side fix for that issue. This relates directly to the changeset in src/aleph.rs, src/bootupd.rs, and src/component.rs.
Full details: Commit Message Convention
-
Explanation
The PR contains one non-merge commit. Its subject is
aleph: handle missing or null version field, which uses a valid component prefix, starts the description with the lowercase imperativehandle, and has no trailing period.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 11 functions across 3 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
|
Johan-Liebert1
left a comment
Johan-Liebert1
left a comment
There was a problem hiding this comment.
LGTM. We don't require bootc-dev/bootc#2591 with this in place right?
Thanks. Replied there |
|
CI should be fixed by #1179 |
In bootc [1] already defines the version field as Option<String>, so we should do the same. This handles the field being unset, or set to null as observed in [2]. This change is the "reader" side fix mentioned in that issue. [1]: https://github.com/bootc-dev/bootc/blob/main/crates/lib/src/install/aleph.rs [2]: coreos#1166
df13ef5 to
3a51894
Compare
In bootc https://github.com/bootc-dev/bootc/blob/main/crates/lib/src/install/aleph.rs already defines the version field as Option, so we should do the same. This handles the field being unset, or set to null as observed in #1166. This change is the "reader" side fix mentioned in that issue.