Skip to content
This repository has been archived by the owner on Sep 4, 2019. It is now read-only.

Adding documentation on conventions for Analyzers. #362

Merged
merged 5 commits into from Sep 3, 2015

Conversation

jmarolf
Copy link
Contributor

@jmarolf jmarolf commented Aug 18, 2015

This adds the details about analyzer conventions in nuget packages

@dnfclas
Copy link

dnfclas commented Aug 18, 2015

Hi @jmarolf, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;


$/analyzers/{**Framework Name**}{**Version**}/{**Supported Architecture**}/**{Supported Programming Language**}/{**Analyzer**}.dll

**Framework Name and Version** the *optional* API surface area of the .NET framework that the contained dlls need to run. If no target is specified, dlls are assumed to apply to *all* targets
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand how you can drop one of the items in the path, but not how you can drop multiple of them, and somehow figure out what was dropped.

Either we say that FrameworkName is All (for all frameworks), and similarly for versions/architecture
or we have a very complicated resolution rules.

Can you please elaborate?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is no folder, we assume that the current host can load the dll. If the user places the dll in a folder whose framework is unsupported or unknown, we skip loading the dll. If the framework is known and compatible, we load the dll.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you just have cs, is that a language or a framework? There is no way to tell other than hardcoding cs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not anticipate there being a "cs" framework. I am ok special casing "cs", "vb", and "fs". Under what circumstance would this happen?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, lets come up with exact samples of what collapsing scenarios are supported, and take it from there.

Other than that this looks really good.

@yishaigalatzer
Copy link
Contributor

CC @hmemcpy

@yishaigalatzer
Copy link
Contributor

One pretty major thing I see that is missing, is how to build a package that works both for packages.config scenarios and project.json scenario. I think you made a mix of msbuild targets/props and install.ps1 to solve it. It is advisable to show the end users how to build it.

@jmarolf
Copy link
Contributor Author

jmarolf commented Aug 21, 2015

ping @yishaigalatzer

@yishaigalatzer
Copy link
Contributor

Thanks for the update and sorry I missed the fact that you pushed a new one. Added some comments, overall looks close

@yishaigalatzer
Copy link
Contributor

@jmarolf ping

Explicitly stating what happens when framework names and programming
language names collide.

Adding links to nuget.org.

Changing wording to make sure the user understands that they need to use
install.ps1 and uninstall.ps1 if they want their analyzer to load outside
of project.json scenarios.
@jmarolf
Copy link
Contributor Author

jmarolf commented Sep 3, 2015

@yishaigalatzer Is there anything else you want added that was not in the last two commits? Anything worded improperly?

yishaigalatzer pushed a commit that referenced this pull request Sep 3, 2015
Adding documentation on conventions for Analyzers.
@yishaigalatzer yishaigalatzer merged commit d0459ef into NuGet:master Sep 3, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants