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

use _yield from []_ to create empty generator when needed #2572 #2581

Merged
merged 13 commits into from
Feb 3, 2025

Conversation

dhruvak001
Copy link
Contributor

@dhruvak001 dhruvak001 commented Jan 29, 2025

The issue#2572 arises from an old trick of making a function behave as a generator by using if False: yield .... The correct and idiomatic solution is to replace such patterns with yield from [], which is clean, efficient, and easy to understand.

  • No CHANGELOG update needed
  • No new tests needed
  • No documentation update needed

Copy link

google-cla bot commented Jan 29, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Please add bug fixes, new features, breaking changes and anything else you think is worthwhile mentioning to the master (unreleased) section of CHANGELOG.md. If no CHANGELOG update is needed add the following to the PR description: [x] No CHANGELOG update needed

@github-actions github-actions bot dismissed their stale review January 29, 2025 15:32

CHANGELOG updated or no update needed, thanks! 😄

@dhruvak001 dhruvak001 closed this Jan 29, 2025
@dhruvak001 dhruvak001 reopened this Jan 29, 2025
Copy link
Contributor Author

@dhruvak001 dhruvak001 left a comment

Choose a reason for hiding this comment

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

use yield from [] to create empty generator when needed #2572

@williballenthin
Copy link
Collaborator

the old pattern was originally used to support Python 2.7 which didn't have yield from .... Now that we're safely beyond that version, this is a much better solution.

@williballenthin
Copy link
Collaborator

@dhruvak001 are there any other parts of the codebase with a similar pattern that needs updating?

@dhruvak001
Copy link
Contributor Author

@williballenthin i don't think so as far as I have seen.

@dhruvak001 dhruvak001 marked this pull request as draft January 30, 2025 18:57
@dhruvak001 dhruvak001 marked this pull request as ready for review January 30, 2025 19:01
@dhruvak001
Copy link
Contributor Author

@williballenthin can u please review it?

Copy link
Collaborator

@williballenthin williballenthin left a comment

Choose a reason for hiding this comment

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

looks good. please address the linter failures.

@dhruvak001
Copy link
Contributor Author

mandiant/capa-rules#988

@williballenthin plz check out the above PR in capa-rules for linter failures update.

@williballenthin
Copy link
Collaborator

code style/black is failing. would you please fix the python file formatting?

@dhruvak001
Copy link
Contributor Author

All Done @williballenthin

Copy link
Collaborator

@williballenthin williballenthin left a comment

Choose a reason for hiding this comment

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

please update the submodule back to master@capa-rules can we can merge this.

@dhruvak001
Copy link
Contributor Author

@williballenthin updated the submodule to the again the master @capa-rules, but again the rule linter check failed.

@williballenthin
Copy link
Collaborator

but again the rule linter check failed

thats expected at the moment and won't be a blocker here. the issues are unrelated to the changes in this PR.

Copy link
Collaborator

@williballenthin williballenthin left a comment

Choose a reason for hiding this comment

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

thank you @dhruvak001!

@williballenthin williballenthin merged commit 923e5e1 into mandiant:master Feb 3, 2025
8 of 9 checks passed
@williballenthin
Copy link
Collaborator

our linters fail in master due to the hex casing changes in this PR:
https://github.com/mandiant/capa/actions/runs/13115215108/job/36587777073?pr=2586

i will revert those changes

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