Making Glimpse CLS-compliant. #565

Merged
merged 4 commits into from Oct 24, 2013

Conversation

Projects
None yet
2 participants
@kellystuard
Contributor

kellystuard commented Oct 13, 2013

Pulling in some additional information, to [hopefully] make the code review easier.

CA1014: Mark assemblies with CLSCompliantAttribute
"The Common Language Specification (CLS) defines naming restrictions, data types, and rules to which assemblies must conform if they will be used across programming languages. Good design dictates that all assemblies explicitly indicate CLS compliance with CLSCompliantAttribute."
"If you do not want the assembly to be compliant, apply the attribute and set its value to false."

Making Your Code CLS Compliant
"If you mark an assembly as CLSCompliant and if any of the publicly exposed types of members are not CLS compliant then the compiler would raise an error"

What is the 'CLSCompliant' attribute in .NET?
"If you intend your code to be consumed by other developers, your API (your public classes and methods) should be CLS compliant."

Effective C#: 50 Specific Ways to Improve Your C# Pages 181-186 (Item 30)
"The payback of interlanguage operability is worth the extra time."

Any reason not to mark a DLL as CLSCompliant?
"There are several C# features that are not CLS compliant, for example unsigned types. Since several languages are case insensetive, there may not be classes and members that are differ only by case, MyObject and myObject, and several other features."
"CLS compliance is probably of most importance to library authors, who can't control who the caller is."

@kellystuard

This comment has been minimized.

Show comment
Hide comment
@kellystuard

kellystuard Oct 13, 2013

Contributor

Previous comment, from @CGijbels: "Adding the [assembly: CLSCompliant(true)] might solve the CA rule from failing, but that doesn't mean the assembly suddenly becomes CLS Complient 😉"

Contributor

kellystuard commented Oct 13, 2013

Previous comment, from @CGijbels: "Adding the [assembly: CLSCompliant(true)] might solve the CA rule from failing, but that doesn't mean the assembly suddenly becomes CLS Complient 😉"

@kellystuard

This comment has been minimized.

Show comment
Hide comment
@kellystuard

kellystuard Oct 13, 2013

Contributor

I have confirmed that if you write code that is not CLS-compliant and then add the attribute, the compiler will tell you about it. Congratulations on writing good code that also does not break any of the CLS specifications.

Code that uses CLS-compliant code does not have to be CLS-compliant. So, any 3rd-party tabs will continue to function, with this change; as will any applications that use Glimpse.

Contributor

kellystuard commented Oct 13, 2013

I have confirmed that if you write code that is not CLS-compliant and then add the attribute, the compiler will tell you about it. Congratulations on writing good code that also does not break any of the CLS specifications.

Code that uses CLS-compliant code does not have to be CLS-compliant. So, any 3rd-party tabs will continue to function, with this change; as will any applications that use Glimpse.

@avanderhoorn

This comment has been minimized.

Show comment
Hide comment
@avanderhoorn

avanderhoorn Oct 14, 2013

Member

In principle I don't have any problems with this update, just wondering what tangible differences we will get beyond the semantic? Also, for this change to have any real effect, you will need to update the AssemblyInfo's in the "Legacy Support" solution folder.

Member

avanderhoorn commented Oct 14, 2013

In principle I don't have any problems with this update, just wondering what tangible differences we will get beyond the semantic? Also, for this change to have any real effect, you will need to update the AssemblyInfo's in the "Legacy Support" solution folder.

@kellystuard

This comment has been minimized.

Show comment
Hide comment
@kellystuard

kellystuard Oct 14, 2013

Contributor

I see two important differences:

  1. Should you ever put in code that would break CLS-compliance, the compiler would error out and not let it happen. This means other .NET languages, besides C#, would not have issues. If this attribute was not applied, no compiler warning or error would be produced, should non-compliant code be put in.

  2. People who are marking themselves as CLS-compliant are getting warnings every time they use a line of code, related to Glimpse. I don't like running around with warnings.

An example of 1) public uint Count { get; set; }
An example of 2) @kellystuard

Contributor

kellystuard commented Oct 14, 2013

I see two important differences:

  1. Should you ever put in code that would break CLS-compliance, the compiler would error out and not let it happen. This means other .NET languages, besides C#, would not have issues. If this attribute was not applied, no compiler warning or error would be produced, should non-compliant code be put in.

  2. People who are marking themselves as CLS-compliant are getting warnings every time they use a line of code, related to Glimpse. I don't like running around with warnings.

An example of 1) public uint Count { get; set; }
An example of 2) @kellystuard

@kellystuard

This comment has been minimized.

Show comment
Hide comment
@kellystuard

kellystuard Oct 14, 2013

Contributor

I will happily update each "Legacy" solution and modify this pull request. Are you alright with every solution being marked?

Contributor

kellystuard commented Oct 14, 2013

I will happily update each "Legacy" solution and modify this pull request. Are you alright with every solution being marked?

@kellystuard

This comment has been minimized.

Show comment
Hide comment
@kellystuard

kellystuard Oct 17, 2013

Contributor

Based on feedback, I'm considering this complete. Please let me know if there's anything else I can do to help.

Contributor

kellystuard commented Oct 17, 2013

Based on feedback, I'm considering this complete. Please let me know if there's anything else I can do to help.

@avanderhoorn

This comment has been minimized.

Show comment
Hide comment
@avanderhoorn

avanderhoorn Oct 21, 2013

Member

We have reviewed this and I'm going to bring it in :) Thanks!

As a note, we have to update FX cop to tap into the CLS compliance stuff at the correct level we want to look at it with.

Member

avanderhoorn commented Oct 21, 2013

We have reviewed this and I'm going to bring it in :) Thanks!

As a note, we have to update FX cop to tap into the CLS compliance stuff at the correct level we want to look at it with.

avanderhoorn added a commit that referenced this pull request Oct 24, 2013

@avanderhoorn avanderhoorn merged commit 0bbdb0f into Glimpse:master Oct 24, 2013

@ghost ghost assigned avanderhoorn Oct 24, 2013

@kellystuard kellystuard deleted the kellystuard:clscompliant branch Feb 10, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment