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

[.NET] Add tests for GodotDisabledSourceGenerators #88687

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

paulloz
Copy link
Member

@paulloz paulloz commented Feb 22, 2024

@paulloz paulloz requested a review from a team as a code owner February 22, 2024 22:18
@paulloz paulloz added this to the 4.x milestone Feb 22, 2024
@paulloz paulloz force-pushed the dotnet/test-generator-disabling branch 3 times, most recently from f0a6656 to ea9f8fc Compare February 25, 2024 20:57
Copy link
Member

@raulsntos raulsntos left a comment

Choose a reason for hiding this comment

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

Thanks for adding tests for the GodotDisabledSourceGenerators property.

var verifier = new Test();

verifier.TestState.AnalyzerConfigFiles.Add(("/.globalconfig", $"""
verifier.TestState.AnalyzerConfigFiles.Add((Constants.GlobalConfig, $"""
Copy link
Member

Choose a reason for hiding this comment

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

Not sure that this needs to be constant since we only use it once.

Copy link
Member Author

Choose a reason for hiding this comment

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

We might want to write different analyzers later on, tho.

Copy link
Member

Choose a reason for hiding this comment

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

I think we should only add it when we need it then. But it's also not a big deal, we can keep it.

@paulloz paulloz force-pushed the dotnet/test-generator-disabling branch from ea9f8fc to 1797d98 Compare March 30, 2024 14:07
@paulloz
Copy link
Member Author

paulloz commented Mar 30, 2024

@raulsntos the style check is inconsistent with the guidelines:

Do not use a space:

  • Within single line initializer braces.

Should we change the guidelines, or the check? I'm generally more inclined to add spaces, but removed them in my last commit after recently re-reading the doc page.

@raulsntos
Copy link
Member

We seem to have moved away from that guideline, probably because by the default VSCode adds spaces. So I'd say we should update the guidelines.

@paulloz paulloz force-pushed the dotnet/test-generator-disabling branch from 1797d98 to a803774 Compare March 30, 2024 17:23
@paulloz paulloz force-pushed the dotnet/test-generator-disabling branch 2 times, most recently from 11de997 to ed98461 Compare June 23, 2024 09:16
@paulloz
Copy link
Member Author

paulloz commented Jun 23, 2024

Brought this up to date, and removed everything not related to those new tests.

@paulloz paulloz force-pushed the dotnet/test-generator-disabling branch from ed98461 to 8673157 Compare June 24, 2024 05:29
@paulloz paulloz force-pushed the dotnet/test-generator-disabling branch from 8673157 to e41feb3 Compare August 17, 2024 08:16
@paulloz
Copy link
Member Author

paulloz commented Aug 17, 2024

Just rebased on dev. Would be nice to have that one merged now that we're out of the 4.3 rush 🙏

@raulsntos raulsntos modified the milestones: 4.x, 4.4 Aug 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants