-
Notifications
You must be signed in to change notification settings - Fork 394
Added *.slnx with Xml conditions to OperationConfigDefault. #8863
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
Conversation
|
@dotnet-policy-service agree |
|
CI seems to pass though, even for OSX, so not sure why it is such as problem locally for me. |
Forgind
left a comment
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.
/cc: @edvilme (slnxpert)
I'm not really sure why it would be failing for you locally...best guess is some random configuration thing or out-of-date files not getting updated properly. I tend to trust CI to have a nice, clean environment prior to running tests. |
test/Microsoft.TemplateEngine.TestTemplates/test_templates/TemplateWithConditions/test.slnx
Show resolved
Hide resolved
test/Microsoft.TemplateEngine.TestTemplates/test_templates/TemplateWithConditions/test.slnx
Show resolved
Hide resolved
edvilme
left a comment
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.
Due to the failed tests I'm not even sure if it gets to running the test I added in End2EndTests.cs.
Update: From this and the comments above, this leads me to believe the test was possibly not run correctly
I tried to build this on Windows now as well and got another error (this happens both in my fork and if I try the this repository so it doesn't seem to be related to my changes). Maybe this is is something that changed since I got the latest .NET10 preview? It didn't have this warning for other any of the other target frameworks. Did a quick (local) fix for that though and it builds and my tests passes on Windows so it seems to be something weird with building on my Mac. I can create a seperate issue for that if needed but it passes CI on OSX so I would say that my PR is probably fine. |
edvilme
left a comment
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, thanks!
Problem
*.slnx files doesn't support conditional processing as described in #8784
Solution
Added *.slnx with
ConditionalType.XmlinOperationConfigDefault.cs.Checks:
I've added tests, but I'm running into some problems.
mainbranch usingbuild.sh, I'm getting a ton of IDE0055 errors all over the project.Microsoft.TemplateEngine.Edge.UnitTests, still inmain.End2EndTests.cs.