Recommended changes resulting from automated audit
Closed this issue · 9 comments
We performed an automated audit of your Cake addin and found that it does not follow all the best practices.
We encourage you to make the following modifications:
- You are currently referencing Cake.Core 0.26.0. Please upgrade to 0.28.0
- Your addin should target netstandard2.0. Please note that there is no need to multi-target, netstandard2.0 is sufficient.
Apologies if this is already being worked on, or if there are existing open issues, this issue was created based on what is currently published for this package on NuGet.org and in the project on github.
With recent discussions multi-targetting is again way to go. So this addin now multi-targets full framework 4.6.1 and .NET standard 2.0
I was not aware the recommendation had changed. What’s the reasoning for this change?
Was this discussed on Gitter? I must have missed this discussion.
@Jericho I cannot find something "official" from Microsoft, but there were recent discussion about this on Twitter and they also mentioned it on a .NET Conf talk. The issue is that the support for .NET Standard in 4.6.1 was added retroactively and unfortunately can lead to issues. Also 4.7.1 has issues. Which basically means that you're required to have 4.7.2 installed to get .NET Standard assemblies running. As Cake itself can run on 4.6.1 best is therefore to multi-target 4.6.1 and .NET Standard 2.0.
I just created a PR to update the guidelines: cake-contrib/Home#55
Have you had a discussion about this with the Cake team? I know that @mholo65 has done a lot of work on this, does he agree with your conclusion?
To be clear, I’m not questioning your reasoning. I just want to make sure everybody is on same page and agrees on this change before I modify the Audit tool to enforce this new requirement.
(By the way thanks for the PR on the recommendation document)
Yes, we discussed this inside the Cake team, also if we want to target .NET 4.7.1 with Cake, but we decided to stay on 4.6.1 for Cake and suggest to multi-target for addins. The thing is that this is something which we cannot fix inside Cake, as it is an issue with .NET Framework, and also Microsoft cannot fix easily without introducing a whole bunch of other issues.
I also just created a PR for adding a note about this on the Cake website: cake-build/website#579, and we would also like to do a blog post.
Unfortunately there's no official documentation about this form Microsoft to easily link to.
But they mention it on this .NET Conf presentation starting at 44:48: https://channel9.msdn.com/Events/dotnetConf/2018/S107
And talk about the issue in this tweet: https://twitter.com/terrajobst/status/1031999730320986112
I just read the twitter thread you linked and I now understand the situation: Cake currently is built with .net 4.6.1 which is not 100% compatible with netstandard2.0 (despite some Microsoft documentation). Therefore addins shouldn’t target netstandard2.0 exclusively.
I guess it’s time to modify the audit tool and notify the 200+ addins authors!!!
Just thinking out loud: for the last several months we have been telling addin authors they only need to target netstandard2.0, now we are telling them they need to multi-target and when Cake upgrades to net 4.7.x, we will again tell them to only target netstandard2.0
I'm concerned addin authors will get fatigued that we keep changing the recommendation and will stop listening to our recommendations.