-
Notifications
You must be signed in to change notification settings - Fork 5
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
Change the calculation of the TFM for Cake addins #116
Comments
Do we have examples of Cake addins that don't reference In fact, what is the definition of a To me, it's a class library that has tailor-made aliases specifically to be used in Cake builds, by adding extension methods to Similarly, The fact Cake can leverage/load any assembly via the Finally my understanding is that the TFM rules exist to help ensure that an addin/module will be compatible with Cake runners. If a Cake-Contrib project is not "loaded" by the Cake runners (and therefore doesn't need |
The point came up when adding CakeContrib.Guidelines to Cake.Email and Cake.Email.Common. And while I do agree to the "not caring about not-addins", I think there is probably a reason to reference CakeContrib.Guidlines and that it is probably because a use in a Cake context is intended. So, looking at it that way, for a "common" project that should be used in other cake related projects - shouldn't that "common" project target the same targetFramworks (or a superset) as the intended using packages? |
If it has no direct reference Core, Common nor runners then it's more a case of Cake supporting it than the other way around. Plus what @augustoproiete said. |
We are trying to encourage addin authors to upgrade their addins to target Cake 1.0.0, therefore in my mind it makes sense that the "default" target rule would be "Cake > 1.0.0". |
I kind of agree to both "positions" currently.
Additionally, to address what @devlead said I propose to extend #98 to raise a warning if the project is named either |
A human looking at The entry-point is the Now if you wanted to - as part of analyzing a |
Hmm I don't think rules should be based on the project name. I think we have better ways to determine if the project is an addin/module/recipe, to then recommend the right TFM and Cake.Core/Cake.Common version to depend on if it's a Cake extension. |
Any ideas that work from msbuild are highly welcome 😄 |
Would it work if it was an MSBuild task that runs after the build, and analyzes the generated assemblies via Mono.Cecil (or similar)? |
Probably. I have created #120 to track that. |
I thought more about this and realized that my concern about with using project names to detect what kind of Cake extension it is was mainly because it's hard to differentiate between Cake addins and other Cake supporting projects just based on project names - which is not the case for Modules and Recipes where using the project name would prob. be fine. If we check for
Would that be easier to implement with the current codebase, without going the path I suggested with Mono.Cecil, etc? |
This seems like a sensible approach to me! We can always enhance the detection rules later, but this would be a solid first pass. |
Yes, that's only a real small change compared to the current codebase. I'll proceed with that change and leave #120 open to be revisited later. |
This is mainly a restructured calculation of the project type (addin/module/recipe) and a small change to the default TFM.
This is mainly a restructured calculation of the project type (addin/module/recipe) and a small change to the default TFM.
(GH-116) updated the calculation for TFM
🎉 This issue has been resolved in version 1.0.0 🎉
|
In versions up until Cake 1.0.0 we has only one rule for TargetFramework.
In current development, there is three of them:
netstandard2.0
(required) andnet461
(suggested)netstandard2.0
(required) andnet461
(suggested) andnet5.0
(suggested)netstandard2.0
(required)Currently for addins not referencing Cake (Core or Common) the same rule as for Cake < 1.0.0 is selected.
@Jericho asked in #107
And my reply was
The question is: Does someone have other ideas, or opinions on this? I really don't have a strong opinion on this, the way it is currently implemented was only the first thing that came to my mind when implementing this.
The text was updated successfully, but these errors were encountered: