search

cake-contrib / Home

:house: This is the hub for all the projects that are part of the Cake Contrib Organization.
https://cakebuild.net/community/cake-contrib
MIT License
13 stars 8 forks source link

Make sure StyleCop is added to all projects #11

Open gep13 opened 7 years ago

pascalberger commented 7 years ago

@gep13 Would Cake.CodeAnalysisReporting be interesting for creating reports (not only for StyleCop but for any analyzer / MSBuild warning) in Cake.Recipe?

gep13 commented 7 years ago

@pascalberger yes, would be very interested in adding this in :+1:

pascalberger commented 7 years ago

@gep13 See https://github.com/cake-contrib/Cake.Recipe/pull/93

pascalberger commented 7 years ago

Related to this, it would also be nice to provide a common ruleset for addins and for test projects.

gep13 commented 7 years ago

@pascalberger I think that would be a great idea. Where would you see that ruleset living? And how to get it into each project?

nils-a commented 4 years ago

Can't rulesets be published as nuget-packages? So we'd simply need to reference them? (The reference can then be tested for in the audit..)

gep13 commented 4 years ago

@nils-a short answer is I don’t know. If it is possible, I think this would be a great addition!

pascalberger commented 4 years ago

Can't rulesets be published as nuget-packages? So we'd simply need to reference them? (The reference can then be tested for in the audit..)

In theory this is possible. In reality I don't know of any way which isn't full of pitfalls. In packages.config you had the issues that you need to reference the ruleset in the restored packages folder which contains the version. You could workaround this by adding a PowerShell script to the NuGet package which copies the ruleset to a defined location. In packagereference world PowerShell is no longer supported, packages are restored to a global package cache by default, which makes it even harder.

nils-a commented 4 years ago

Hm. I was under the impression that "nowadays" no more powershell-voodoo was needed.

I created a simple test package:

All worked in my tests.

Maybe i am missing something. :-)

My code is here: https://github.com/nils-a/test-common-analyzer

testing can be done from: https://www.myget.org/feed/nils-a/package/nuget/test-common-analyzer

gep13 commented 4 years ago

@nils-a @pascalberger I was introduced to this project:

https://github.com/endjin/Endjin.RecommendedPractices.NuGet

And I was wondering if this could be useful here as well.

nils-a commented 4 years ago

Nice. Really elaborate. I like it.

@gep13 Did you see they are shipping a default-icon in the package, too? (I feel that's a nice alternative to the git-submodule/always-copy-on-build discussion)

(I'm currently on vacation with really cappy wi-fi, but I'd be willing to start a sample project if that's ok with you all..)

gep13 commented 4 years ago

@nils-a yes, I did notice that. I really like the idea of shipping the icon as a nuget package, and allowing it to be placed into the correct location, and updating of the necessary information.

If you wanted to start the work on this, I would be happy for you to do this.

nils-a commented 4 years ago

@gep I started out with the (allegedly) easy task of providing a simple package to distibute the icon. Check out the work at https://github.com/nils-a/cake.cake-contrib-icon. Currently there is no magic set PackageIcon automatically (See https://github.com/nils-a/cake.cake-contrib-icon/issues/1 for that)

For the common "styleguide" I propose starting a repository (cake.cake-contrib-codestyles ?) in which we could start discussing what rules/styles to use. (for example in Cake.7zip I used stylecop & fxcop, but I also added an .editorconfig for the styling and a ruleset (because setting rules in .editorconfig is only supported starting with VS2019), as well as R#-settings)

gep commented 4 years ago

@gep13 ^^^

Jericho commented 4 years ago

When this package is published, I volunteer to modify AddinDiscoverer to submit a PR to all 300+ addins so they reference this new package.

gep13 commented 4 years ago

@nils-a thanks for looking into this! I think we should start small with anything that we do as a NuGet package that contains the best practices for Cake-Contrib addins. To that end, if we can get a single NuGet package, which only adds the contained Cake-Contrib icon as an embedded icon to the project that it is being added to, I think it would be great to ship that as an initial release of the NuGet package.

Are you at that point?

nils-a commented 4 years ago

@gep13 somewhat, yes. I'm not 100% satisfied with the current status of https://github.com/nils-a/cake.cake-contrib-icon but it's enough for a 0.1.0 release. I'll try to manage a release later today.

gep13 commented 4 years ago

@nils-a that sounds great! I think it would make sense to come up with a name that we can use going forward.

@AdmiringWorm has recently created the CakeContrib.Analyzer project (https://github.com/AdmiringWorm/CakeContrib.Analyzer) so I think a name like CakeContrib.Guidelines would make sense.

Thoughts?

/cc @pascalberger @Jericho

pascalberger commented 4 years ago

@AdmiringWorm has recently created the CakeContrib.Analyzer project (https://github.com/AdmiringWorm/CakeContrib.Analyzer) so I think a name like CakeContrib.Guidelines would make sense.

Oh, analyzers for Cake addins? Nice 👍 But wouldn't it make sense to make them not scoped to cake-contrib, but for any Cake addin (regarding the cake-contrib in the name)? Maybe something which we once can make part of the cake-build organization?

Thoughts?

For a package enforcing guidelines for the cake-contrib organization I think CakeContrib.Guidelines makes sense.

gep13 commented 4 years ago

@pascalberger said... Maybe something which we once can make part of the cake-build organization?

That is an interesting question. I can't speak for @AdmiringWorm here, but I think the naming was more to do with the fact that any Cake Addin is part of the extended Cake Contrib (i.e. the Cake Community) regardless of whether it was contained within the Cake-Contrib Organisation on GitHub.

AdmiringWorm commented 4 years ago

Speaking on my own behalf, the reason I went with CakeContrib as the name is because the end-goal of the analyzer is to recommend/enforce a common set of rules that Cake Contrib addins are expected to adhere to (once such rules have been agreed upon).

With that said though, currently there are only two rules available in the analyzer, and none of them are specific to Cake Contrib addins, so it could very well be renamed to more of a general set of recommendations for all addins wether they are in Cake Contrib or not (currently only rules for missing CakeMethodAlias/CakePropertyAlias and missing CakeAliasCategory attributes are implemented, couldn't think of any more).

pascalberger commented 4 years ago

IMHO it would be nice to have one project which provides rules for writing Cake addins, which is not related to cake-contrib projects. And another project which provides a default ruleset for cake-contrib projects (which would be CakeContrib.Guidelines with the above suggestion)

AdmiringWorm commented 4 years ago

Fair enough, I don't really disagree on the point of having a analyser for addins not related to cake-contrib. If push comes to shove, a separate analyser could be created if needed more specific to cake-contrib that pulls in the more general one as a dependency (as such it would be more or less the same).

Regarding the suggestion of CakeContrib.Guidelines mentioned, to me that seems more geared towards the metadata specified in project files, and not really what analysers provide (which is static analysing of the source files themselves, but not on project files. At least not natively supported by the API provided by Microsoft).

A small note on the analyser I am working on, it only supports MSBuild 16.0+ (VS2019), since there were some APIs not available in earlier versions of the CodeAnalyzer library that I wanted (forgot which ones though).

nils-a commented 4 years ago

To be clear (in other words: sometimes I'm a bit slow):

or is that one package and we're simply starting by "enforcing" the icon?

gep13 commented 4 years ago

@nils-a I think we should have a single package called CakeContrib.Guidelines. And since using an embedded icon is one of our guidelines, we should start with getting that deployed as part of the first version of this package.

nils-a commented 4 years ago

So I've created https://github.com/nils-a/CakeContrib.Guidelines and the 0.1.0-release is available at https://www.nuget.org/packages/CakeContrib.Guidelines/.