-
Notifications
You must be signed in to change notification settings - Fork 228
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
Split CleanBinaries target #9357
Conversation
<BinariesToDelete Include="$(BinariesFolder)\Google.Protobuf.dll" /> | ||
<BinariesToDelete Include="$(BinariesFolder)\SonarAnalyzer.dll" /> | ||
<BinariesToDelete Include="$(BinariesFolder)\SonarAnalyzer.CFG.dll" /> |
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.
This will collide with VB and doesn't solve the problem of deleting files that should no longer be there. We'll need to create subdirs per analyzer if you want to split this.
The deletion needs to solve the problem of SonarAnalyzer.dll
was renamed to SonarAnalyzer.Something.dll
but local packaging is still, by accident, or funky checked out commit, picking up the old file.
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.
If we change the file structure we need to update the consumers. Is it only the java plugin?
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.
There is also this solution but it requires a lot more scaffolding. On the other hand, it would probably the cleanest.
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.
Java plugin, NuSpec files, .NET ITs AFAICT. Search for the well-known filenames in the repo should do the trick of finding what I missed
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.
Such "helper projects" will cause more confusion and might introduce some troublesome dependencies.
@@ -9,9 +9,4 @@ | |||
<BinariesFolderInternal>$(BinariesFolder)internal\</BinariesFolderInternal> | |||
</PropertyGroup> | |||
|
|||
<!-- The condition causes the target to only be executed once during the outer build step. |
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.
The idea with setting a flag here didn't work?
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.
What was the exact idea? I remember trying something and it didn't work.
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.
Basically add Condition="$(SomeCustomFlag)!='True'"
(wrong syntax, just illustrative) and then inside the target
<PropertyGroup><SomeCustomFlag>True</...
.
That will try to execute it just once. I'm not sure if it will work as expected. That should be the smallest change. The need for maintaining this on many different files is scattered => hard to keep track of.
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.
No, it does not work. The flag is not persisted.
https://trello.com/c/0QOHiwL1/1360-future-proof-cleanbinaries-target