Skip to content
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

Make sure StyleCop is added to all projects #11

Open
gep13 opened this issue Apr 14, 2017 · 26 comments
Open

Make sure StyleCop is added to all projects #11

gep13 opened this issue Apr 14, 2017 · 26 comments

Comments

@gep13
Copy link
Member

gep13 commented Apr 14, 2017

No description provided.

@pascalberger
Copy link
Member

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

@gep13
Copy link
Member Author

gep13 commented Apr 20, 2017

@pascalberger yes, would be very interested in adding this in 👍

@pascalberger
Copy link
Member

@pascalberger
Copy link
Member

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

@gep13
Copy link
Member Author

gep13 commented Apr 24, 2017

@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
Copy link
Contributor

nils-a commented Jul 1, 2020

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
Copy link
Member Author

gep13 commented Jul 1, 2020

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

@pascalberger
Copy link
Member

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
Copy link
Contributor

nils-a commented Jul 1, 2020

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

I created a simple test package:

  • it references StyleCop.Analyzers
  • deploys a custom ruleset
  • deploys a custom stylecop.json (for file-headers and such..)

All worked in my tests.

  • I set SA1600 to error. My Test shows:
    image
  • I added some Information before build:
    image

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
Copy link
Member Author

gep13 commented Aug 19, 2020

@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
Copy link
Contributor

nils-a commented Aug 20, 2020

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
Copy link
Member Author

gep13 commented Aug 20, 2020

@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
Copy link
Contributor

nils-a commented Aug 21, 2020

@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
Copy link

gep commented Aug 22, 2020

@gep13 ^^^

@Jericho
Copy link
Member

Jericho commented Aug 25, 2020

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
Copy link
Member Author

gep13 commented Aug 26, 2020

@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
Copy link
Contributor

nils-a commented Aug 26, 2020

@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
Copy link
Member Author

gep13 commented Aug 26, 2020

@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
Copy link
Member

@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
Copy link
Member Author

gep13 commented Aug 26, 2020

@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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Contributor

nils-a commented Aug 27, 2020

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

  • The icon will be a separate package (CakeContrib.Icon, then?)
  • The guidelines (containing a reference to stylecop including rules etc) will be named CakeContrib.Guidelines

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

@gep13
Copy link
Member Author

gep13 commented Aug 28, 2020

@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
Copy link
Contributor

nils-a commented Aug 28, 2020

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/.

  • It carries the current icon and copies it into the project-root on build.
  • It does not (sadly) automatically set the icon as PackageIcon, it does however issue a build-error when PackageIcon is not set and a warning when PackageIcon points to something different than the copied icon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants