(GH-80) existing icons will be honored

[nils-a]

Additionally no local copy of the icon is needed anymore.

The setting $(CakeContribGuidelinesIconDestinationLocation) has
been removed. Any old build/project using this setting will now fail.

fixes #80

What has changed

Activate this on a project with no PackageIcon-property, no referenced icon:
You'll get the default icon as icon.png in the nupkg, automatically. No files added to the project.

Activate this on a project with PackageIcon set to some arbitrary png (e.g. <PackageIcon>my-cool-icon.png</PackageIcon>), but no referenced icon:
You'll get the default icon renamed (e.g. to my-cool-icon.png in the nupkg. Still, no files added to the project.

Activate this on a project with PackageIcon and a corresponding referenced icon (probably the default for all existing packages):
On build the icon will be checked and overridden/updated, if needed.

Additionally:

• PackageIconUrl will automatically be set to https://cdn.jsdelivr.net/gh/cake-contrib/graphics/png/cake-contrib-medium.png if not specified
• CakeContribGuidelinesIconDestinationLocation was removed. This is a breaking change.

[Jericho]

Automatically setting the PackageIconUrl is a very nice touch.

[codecov]

Codecov Report

Merging #107 (a5d451d) into develop (0d4670f) will increase coverage by 7.07%.
The diff coverage is 97.81%.

@@             Coverage Diff             @@
##           develop     #107      +/-   ##
===========================================
+ Coverage    80.39%   87.47%   +7.07%     
===========================================
  Files           11       13       +2     
  Lines          306      431     +125     
  Branches        48       64      +16     
===========================================
+ Hits           246      377     +131     
+ Misses          41       33       -8     
- Partials        19       21       +2     

Impacted Files	Coverage Δ
src/Tasks/CcgLogError.cs	0.00% <ø> (ø)
src/Tasks/CcgLogWarning.cs	0.00% <ø> (ø)
src/Tasks/TargetFrameworkVersions.cs	87.21% <75.00%> (+0.09%)	⬆️
src/Tasks/EnsureCakeContribIcon.cs	97.95% <97.95%> (ø)
src/Tasks/EnsureCakeContribIconUrl.cs	100.00% <100.00%> (ø)
src/Tasks/Extensions/StringExtensions.cs	100.00% <100.00%> (ø)
src/Tasks/RequiredFileEditorconfig.cs	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0d4670f...a5d451d. Read the comment docs.

[nils-a]

Since this is a major overhaul of the CCG001/2/3 detection I'd love to have a couple of eyes on this:
@augustoproiete @Jericho @gep13 @AdmiringWorm is one of you (preferably more that one..) in a position to take this for a spin?

[Jericho]

I can certainly give it a go. Where do you publish beta packages?

[nils-a]

@Jericho They are published "here" in GitHub, however not for PRs.

Interestingly though, Currently there was not even a nupkg uploaded as an artifact in this build.

The package is at https://github.com/cake-contrib/CakeContrib.Guidelines/suites/2217423096/artifacts/45894859

[Jericho]

I just used CakeContrib.Guidelines.0.6.0-gh-80-0001.nupkg on two of my addins:

• it honored my embedded icon icon in both cases, which is great
• it did not complain about the fact that neither of my csproj contain PackageIconUrl
• in one case, the project is targeting netstandard2.0 ONLY and it complained that I am missing net461
  
• in the other case, which is also targeting netstandard2.0 ONLY, it complained that I am missing net461 and net5.0
  

[nils-a]

For CCG0007

• Cake.Email.Common has no reference to Cake.Core - which results in the "default" rule:
  • Required: netstandard2.0
  • Suggested: net461
• Cake.Email references Cake.Core in version 1.0.0 which results in
  • Required: netstandard2.0
  • Suggested: net461 and net5.0

That the rest worked for you I'll take as a

[Jericho]

Why are the targeting rules different depending on whether an addin targets Cake.Core or not? Shouldn't we be consistent and always make the same recommendation?

Also, did you see my comment about that fact that my csproj is missing PackageIconUrl but I did not get a warning? I was under the impression that it would be automatically set.

[nils-a]

@Jericho

did you see my comment about that fact that my csproj is missing PackageIconUrl but I did not get a warning?

I did, sorry for not commenting on that. When no PackageIconUrl is set, it will be automatically set to https://cdn.jsdelivr.net/gh/cake-contrib/graphics/png/cake-contrib-medium.png - there will be no warning generated.

Why are the targeting rules different depending on whether an addin targets Cake.Core or not?

The rules "follow" the Cake version: If you are targeting Cake ≥ 1.0.0 the "requirement" is for netstandard2.0 and suggestions (i.e. warnings) are for net461 and net5.0. If you target Cake < 1.0.0, net5.0 is not suggested because Cake can not handle that.

Now, the question arises what to do in cases where no reference to Cake.Core or Cake.Common is set. Currently the rule defaults to the "lowest" requirement - in this case that is the "Cake < 1.0.0"-rule.
We could argue, that since Cake 1.0.0 is now the default, the default-rule should be the "Cake ≥ 1.0.0"-rule.

[Jericho]

When no PackageIconUrl is set, it will be automatically set to https://cdn.jsdelivr.net/gh/cake-contrib/graphics/png/cake-contrib-medium.png - there will be no warning generated.

This did not work as expected: the value was not automatically set for either of my two addins.

[nils-a]

You are right. Additionally I see two icons packed when packing Cake.Email.Common. Will analyze that.

[nils-a]

The "two icons" being packed was me using a wrong nupkg.
The PackageIconUrl not being set has been fixed.