[ULS] Update for Rocky version agnostic specs - #86
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the spec-update tooling to support Rocky version-agnostic kernel specs by omitting the .elX suffix in changelog entries when the spec does not define el_version.
Changes:
- Update
update_lt_spec.pyto build changelogdistas empty whenel_versionis not defined. - Change
read_spec_el_version()to returnNone(instead of raising) whenel_versionis not present in the spec.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| update_lt_spec.py | Builds changelog entries without .elX when el_version is absent. |
| kt/ktlib/ciq_helpers.py | Makes read_spec_el_version() optional by returning None when not found. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| Returns the el_version string (e.g., '9'), or None if %define el_version | ||
| is not present in the spec. |
There was a problem hiding this comment.
Maybe valid and fair but i'm not going to block on it.
There was a problem hiding this comment.
Updated the docstring
| def read_spec_el_version(spec_lines): | ||
| """Read the EL version number from spec file lines. | ||
|
|
||
| Returns the el_version string (e.g., '9'). | ||
| Raises ValueError if not found. | ||
| Returns the el_version string (e.g., '9'), or None if %define el_version | ||
| is not present in the spec. |
There was a problem hiding this comment.
Its never had test coverage before but @bmastbergen maybe have claude gen up some coverage for possible use cases to?
There was a problem hiding this comment.
some basic tests added
|
Seem fine but the lack of test coverage on |
The 6.18 spec has been updated to work with Rocky 9 or 10 and therefore does not have a hardcoded el_version anymore. Since the spec can be used to build for 9 or 10 it doesn't make sense to encode elX in the changelog at all. So if a spec defines el_version we can still add it (ie. ciq-6.12.y today), but if el_version isn't defined, just leave elX out of the changelog completely.
044cc21 to
489fd63
Compare
The 6.18 spec has been updated to work with Rocky 9 or 10 and therefore does not have a hardcoded el_version anymore. Since the spec can be used to build for 9 or 10 it doesn't make sense to encode elX in the changelog at all. So if a spec defines el_version we can still add it (ie. ciq-6.12.y today), but if el_version isn't defined, just leave elX out of the changelog completely.
Coverage Report