Skip to content

Access sections by both ID and name#531

Merged
centosinfra-prod-github-app[bot] merged 1 commit intomainfrom
sections
Apr 20, 2026
Merged

Access sections by both ID and name#531
centosinfra-prod-github-app[bot] merged 1 commit intomainfrom
sections

Conversation

@nforro
Copy link
Copy Markdown
Member

@nforro nforro commented Apr 8, 2026

Fixes #417.

@centosinfra-prod-github-app
Copy link
Copy Markdown
Contributor

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the Sections class to support lookups by both normalized ID and normalized name in the __contains__ and find methods. A new unit test has been added to verify this behavior. The review feedback suggests refining the __contains__ implementation to safely handle non-string inputs, such as Section objects, to avoid potential AttributeError exceptions when calling .lower().

Comment thread specfile/sections.py Outdated
@nforro nforro moved this from New to In review in Packit pull requests Apr 9, 2026
@nforro
Copy link
Copy Markdown
Member Author

nforro commented Apr 20, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the Sections class in specfile/sections.py to allow checking for section existence and finding sections using both their normalized ID and normalized name. These changes are implemented in the contains and find methods. A new unit test, test_contains_and_getattr, has been added to verify the updated logic. Feedback was provided to remove a redundant type cast in the contains method for better consistency and readability.

Comment thread specfile/sections.py Outdated
@centosinfra-prod-github-app
Copy link
Copy Markdown
Contributor

Signed-off-by: Nikola Forró <nforro@redhat.com>
Assisted-by: Claude Opus 4.6 via Claude Code
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@centosinfra-prod-github-app
Copy link
Copy Markdown
Contributor

@nforro nforro added the mergeit Merge via Zuul label Apr 20, 2026
@centosinfra-prod-github-app
Copy link
Copy Markdown
Contributor

@centosinfra-prod-github-app centosinfra-prod-github-app bot merged commit 6a5efba into main Apr 20, 2026
48 of 51 checks passed
@github-project-automation github-project-automation bot moved this from In review to Done in Packit pull requests Apr 20, 2026
@nforro nforro deleted the sections branch April 20, 2026 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mergeit Merge via Zuul

Projects

Development

Successfully merging this pull request may close these issues.

Whitespace after section header messes up with section namings

3 participants