-
Notifications
You must be signed in to change notification settings - Fork 377
Enable VerifyClosure target to run independently of other validations #6101
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
Enable VerifyClosure target to run independently of other validations #6101
Conversation
|
Didn't you also mention that you needed to update the task to ensure it creates the directory: arcade/src/Microsoft.DotNet.Build.Tasks.Packaging/src/VerifyClosure.cs Lines 388 to 391 in 9915500
|
There is a different issue for that fix: #6090 I tried to minimize amount of changes in Arcade, to be able to pick up only SharedFrameworks package for RC2. Directory is being created with a new target you proposed for runtime repo. |
|
Doesn't this package carry the task and you're already updating it? You can have fewer moving parts and not need a workaround that you have to remove later. |
No, unfortunately, the task is in a different package: |
|
Hmm, I thought that @jkoritzinsky duplicated it, but maybe that's in a later PR. |
|
The duplication is in the new Shared Framework SDK at #5714 that I'm still waiting on reviews on before I can merge in. |
|
Understood (my review too, apologies). I'll leave it to you @NikolaMilosavljevic to choose what you think is the least risky here. |
Jeremy, that PR is for 6.0, right? |
|
Yes that PR is for 6.0, but given the size of the change I'm hoping to get it in early in 6.0 so we can make sure that any issues that come up are solved before we start the release process. |
Fixes: #6098
Related to the following Runtime PR that needs this change to work: dotnet/runtime#41698
We need to run VerifyClosure target independently of other validations. This change updates the target condition to allow this.