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

fix: prefix STJ generated file namespace with global #654

Merged

Conversation

jupjohn
Copy link
Contributor

@jupjohn jupjohn commented Aug 5, 2024

Noticed this today at work where STJ conversion generated code wasn't compiling. This was caused by the namespace looking something like Abc.Def.Abc which is a classic way to break source generators when they're not prefixing namespaces with global::. This should fix the values of both the key & value's Lazy<T> factory.

This PR is crazy big because it shows up in almost every snapshot test. Adding these individually in path mode without zoning out was a real test.

Love ya' work Steve, keep doing what you're doing!

Copy link
Owner

@SteveDunn SteveDunn 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 to me! Many thanks for your help! 👍

@SteveDunn
Copy link
Owner

Noticed this today at work where STJ conversion generated code wasn't compiling. This was caused by the namespace looking something like Abc.Def.Abc which is a classic way to break source generators when they're not prefixing namespaces with global::. This should fix the values of both the key & value's Lazy<T> factory.

This PR is crazy big because it shows up in almost every snapshot test. Adding these individually in path mode without zoning out was a real test.

Love ya' work Steve, keep doing what you're doing!

Many thanks! How did you reset the snapshots? There's a command line script to do that:

.\RunSnapshots.ps1 -v "minimal" -reset

I'll merge down and do that. Thanks again for the contribution!

@SteveDunn SteveDunn merged commit 63d1053 into SteveDunn:main Aug 5, 2024
5 of 6 checks passed
@jupjohn
Copy link
Contributor Author

jupjohn commented Aug 5, 2024

Cheers. I enabled auto verification, then reviewed them when staging in git 👌

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