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

Change throwaway parameter to include out and add test #10438

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Forgind
Copy link
Member

@Forgind Forgind commented Jul 23, 2024

Fixes #10435

Context

#10209 was generous with what it interpreted as a throwaway parameter: underscores were often misinterpreted as throwaway parameters. This changes it to require out _ instead of just _, which should resolve the problem.

Changes Made

Look for out _ instead of just _

Testing

Created a new unit test to distinguish between _ and out _

Notes

Alternative to #10435

I searched github for all instances of out _ in .props, .targets, and .*proj files and found none that seemed problematic, giving me some confidence in this change. I also looked for instances of _ in .targets files and found plenty of examples that were broken with #10209, indicating that that search would've prevented this problem.

@rainersigwald, leaving this as a draft until you've had a chance to try to think of potential problems with this approach. I was satisfied with my github search.

@Forgind Forgind marked this pull request as ready for review August 15, 2024 22:01
@Forgind
Copy link
Member Author

Forgind commented Aug 15, 2024

@rainersigwald,

I haven't come up with any more ways that this is broken; have you?

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.

Property functions with a single _ as an argument return bad results
2 participants