Ignore, but process, conditional macro expansions when updating Release#377
Ignore, but process, conditional macro expansions when updating Release#377nforro merged 1 commit intopackit:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request addresses an issue with handling conditional macro expansions in spec file Release strings by avoiding flattening the parsed nodes. The change in agents/tools/specfile.py is correct and directly solves the problem. The new tests in agents/tests/unit/test_tools.py validate the fix for z-stream branches. I've added one suggestion to enhance test coverage for non-z-stream branches as well, to ensure all code paths affected by this change are validated.
Flattening the nodes of parsed release string removes conditional macro expansions, so it's not possible to reconstruct them. Process them instead, but treat them as simple string literals, we don't need to process their bodies anyway. Signed-off-by: Nikola Forró <nforro@redhat.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request correctly addresses an issue where conditional macro expansions in the Release field were lost during updates. By removing the flattening of parsed nodes from the specfile's Release string, the original structure, including conditional macros, is preserved. The change is simple and effective, replacing the custom _parse_release method with a direct call to ValueParser.parse. The new test cases in test_tools.py are well-designed, confirming that the fix works as expected for various scenarios, including z-stream branches. The implementation is clean and the fix is well-tested.
Flattening the nodes of parsed release string removes conditional macro expansions, so it's not possible to reconstruct them. Process them instead, but treat them as simple string literals, we don't need to process their bodies anyway.