Fixes #4798 #4799
Fixes #4798 #4799
Conversation
|
Ok... stylecop.. love you.... |
Make the reader classes nested inside the fixture.
I have fixed this. StyleCop is your friend and for my work we have rules set to error so they will be fixed before CI kicks in. Initially it is a hassle for developers new to the code, but in the end they all love it as they don't have to do rework. I added a few more changes to my commit:
|
There was a problem hiding this comment.
@OsirisTerje Code looks good. I asked a few questions to see if we need more checks.
src/NUnitFramework/tests/SolutionTests/NuspecDependenciesTests.cs
Outdated
Show resolved
Hide resolved
src/NUnitFramework/tests/SolutionTests/NuspecDependenciesTests.cs
Outdated
Show resolved
Hide resolved
Yes, I know, but you're far better @manfred-brands ! Thanks ! PS: Wish that StyleCop was a Roslyn analyzer with the possibility to auto fix all these things. |
It is and has many CodeFixes |
|
But is that a separate analyzer to be installed, or do we have it already? UPDATE Found it! Thanks @manfred-brands |
|
@OsirisTerje Thanks for the excellent start, I added more functionality:
I think we are done, but as I should not approve things I worked on @stevenaw can you do the final review please? |
There was a problem hiding this comment.
Thanks @OsirisTerje @manfred-brands
LGTM. Seems robust and no real concerns of any note on my end. Passes locally for me. One nitpick suggestion about test naming consistency and clarity
src/NUnitFramework/tests/SolutionTests/NuspecDependenciesTests.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Steven Weerdenburg <stevenaw@users.noreply.github.com>
|
Thanks for accepting the suggestion @OsirisTerje |
Fixes #4798
I followed the idea from @stevenaw and made this into a test.
The test is in folder for SolutionTests, indicating that it doesn't test code, but tests the setup of the projects.
Writing code like this with XML parsing and so is not something I am very fond out, so I did take some "help" from the ChatGPT "friend". The parsing code it did pretty well, but the logic - that was more of a struggle. I believe I have managed to clean up most of that, after NNN iterations, but there certainly some more weirdoes around there. Appreciate some good comments!
The first commit shows the error, so the action should go red.
The next commit will fix that error, and should go green.
The code takes the csproj as the "truth" and checks that the nuspec have the same setup. Right now it says the System.Memory is missing in the .net6.0 dependency group in the nuspec, but we know that is should instead be placed under the .netframework condition in the csproj. Some thinking is still needed from the devs :-)