Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 zm: Fix generated get_all() function #1223

Merged
merged 1 commit into from
Jan 23, 2025
Merged

Conversation

rzvncj
Copy link
Contributor

@rzvncj rzvncj commented Jan 23, 2025

Naming the optional header parameter 'header' several times for different property getters collides with the 'header` local argument of the generated get_all() function. Obvious workaround is don't name your special parameter 'header'.

This is easily fixed by simply wrapping each getter code block generated for get_all() in its own scope.

Naming the optional header parameter 'header' several times for
different property getters collides with the 'header` local
argument of the generated get_all() function. Obvious workaround
is don't name your special parameter 'header'.

This is easily fixed by simply wrapping each getter code block
generated for get_all() in its own scope.
@zeenix
Copy link
Contributor

zeenix commented Jan 23, 2025

Thanks!

@zeenix zeenix merged commit e8ebe3b into dbus2:main Jan 23, 2025
8 checks passed
@zeenix
Copy link
Contributor

zeenix commented Jan 23, 2025

zm: Fix generated get_all() function

Just for future ref: This is the benefit of the change, not the summary of the change itself. IMO commit summary should have the later as it makes it easy to see in the log which commit did what.

@rzvncj
Copy link
Contributor Author

rzvncj commented Jan 23, 2025

Thanks!

Thanks for the lightning-quick review and merge! Also, the commit info suggestion has been noted.

@rzvncj rzvncj deleted the fix-get_all branch January 24, 2025 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants