-
Notifications
You must be signed in to change notification settings - Fork 703
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
Fixes #3572 and some others: Remove analyzers from solution #3600
Conversation
It's a little different in msbuild.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I switch from develop and this PR branches and rebuild on both and it is working as expected without needing to run any scripts. Thanks.
Good to hear. I had tried in a couple of environments as well, but still wanted to be extra-sure I wasn't just getting lucky. 😅 I was originally splitting them off into a local nuget package but decided it wasn't worth wasting the effort on that since I'm already going to be doing something similar but not identical when I get back to it in the formal public nuget package I'll be putting out eventually. Just wanted to work on other more important stuff for now and make your workflows easier in as simple a way as possible. There are other ways to alter your workflow that would have avoided the pain you guys were feeling, but it didn't feel worth altering those habits for something temporary. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Thank you. Thank you. Thank you.
@dodexahedron It looks like you removed the unit tests. It would be beneficial to create them in the |
<ProjectReference Include="..\Analyzers\Terminal.Gui.Analyzers.Internal\Terminal.Gui.Analyzers.Internal.csproj"> | ||
<PrivateAssets>all</PrivateAssets> | ||
<OutputItemType>Analyzer</OutputItemType> | ||
<ReferenceOutputAssembly>false</ReferenceOutputAssembly> | ||
</ProjectReference> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You removed it here in #3600 but it's again on the v2_develop
.
Terminal.Gui/Terminal.Gui/Terminal.Gui.csproj
Lines 62 to 66 in 7dc12a4
<ProjectReference Include="..\Analyzers\Terminal.Gui.Analyzers.Internal\Terminal.Gui.Analyzers.Internal.csproj"> | |
<PrivateAssets>all</PrivateAssets> | |
<OutputItemType>Analyzer</OutputItemType> | |
<ReferenceOutputAssembly>false</ReferenceOutputAssembly> | |
</ProjectReference> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the analyzer project and its tests, yes.
Didn't remove the actual TG UnitTests project.
But good thought on reformatting and whatnot. I'll add what's necessary to make ReSharper ignore those files as part of #3558 too, so it doesn't change things.
This PR removes the enum source generator projects, code, pwsh module, and CI build item from v2.
The generated code from the v2 code at the base of this branch was copied into normal source files so that existing uses can be preserved.
Note that, if you change one of those enums, you need to add an entry to the FastIsDefined method's switch expression for that enum's extension class. Enum changes are considered breaking changes in the .net world, anyway, however.
Those files will get deleted at whatever point in the future I come back to the generators. Probably during beta.
Fixes
Analyzers
-> Restarting VS when simply changing branches is not acceptable. #3477Proposed Changes/Todos
Pull Request checklist:
CTRL-K-D
to automatically reformat your files before committing.dotnet test
before commitI have made corresponding changes to the API documentation (using///
style comments)- Not relevant
Ready to go