Skip to content

Conversation

gilles-peskine-arm
Copy link
Contributor

Fix silly mistake in #196 that clogs up the chain of consumers.

PR checklist

Please add the numbers (or links) of the associated pull requests for consuming branches. You can omit branches where this pull request is not needed.

`output_files` was accidentally an iterator that was not iterated over
unless one of the list-only options was enabled.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
@gilles-peskine-arm gilles-peskine-arm added needs-review Every commit must be reviewed by at least two team members, priority-very-high Highest priority - prioritise this over other review work size-xs Estimated task size: extra small (a few hours at most) labels Sep 16, 2025
output_files = generate_header_files(branch_data,
options.output_directory,
list_only=list_only)
output_files = list(generate_header_files(branch_data,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The fact that we all missed this at first shows that this is not a good way to write the code. But right now I'd rather unblock the chain of PR ASAP.

Copy link
Contributor

@mpg mpg left a comment

Choose a reason for hiding this comment

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

LGTM.

I feel silly for missing this, because I was about to say the function might as well return a list (or tuple) directly, then I thought it's probably a matter of taste, let's not delay the PR over this... but completely missed the practical impact here :(

Copy link
Contributor

@minosgalanakis minosgalanakis left a comment

Choose a reason for hiding this comment

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

LGTM

@github-project-automation github-project-automation bot moved this from In Development to Has Approval in Roadmap pull requests (new board) Sep 16, 2025
@gilles-peskine-arm gilles-peskine-arm added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members, labels Sep 16, 2025
@gilles-peskine-arm gilles-peskine-arm merged commit 53c7928 into Mbed-TLS:main Sep 16, 2025
2 of 4 checks passed
@github-project-automation github-project-automation bot moved this from Has Approval to Done in Roadmap pull requests (new board) Sep 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Design and code approved - may be waiting for CI or backports priority-very-high Highest priority - prioritise this over other review work size-xs Estimated task size: extra small (a few hours at most)
Development

Successfully merging this pull request may close these issues.

3 participants