Skip to content

Enable CLS Compliance on Diagnostics Libraries#89214

Merged
tarekgh merged 1 commit into
dotnet:mainfrom
tarekgh:EnableCLSComplianceOnDiagnosticLibraries
Jul 20, 2023
Merged

Enable CLS Compliance on Diagnostics Libraries#89214
tarekgh merged 1 commit into
dotnet:mainfrom
tarekgh:EnableCLSComplianceOnDiagnosticLibraries

Conversation

@tarekgh

@tarekgh tarekgh commented Jul 19, 2023

Copy link
Copy Markdown
Member

This change will allow the consumers of these libraries not to be forced to set CLS Compliance off.

@ghost ghost added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jul 19, 2023
@tarekgh tarekgh added area-System.Diagnostics and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jul 19, 2023
@tarekgh tarekgh added this to the 8.0.0 milestone Jul 19, 2023
@tarekgh tarekgh requested review from Tratcher and noahfalk July 19, 2023 20:46
@tarekgh tarekgh self-assigned this Jul 19, 2023
@noahfalk

Copy link
Copy Markdown
Member

I assume the affect of this change is that a CLSCompliant attribute at assembly scope changes from false to true (or maybe goes away if that is the default), and now other libraries that reference this one don't get compiler warnings? That sounds pretty good to me.

Are you aware of any scenario where the experience gets worse? I can't think of anything offhand.

@tarekgh

tarekgh commented Jul 19, 2023

Copy link
Copy Markdown
Member Author

I assume the affect of this change is that a CLSCompliant attribute at assembly scope changes from false to true (or maybe goes away if that is the default)

By default, if the CLSCompliant is not showing in the assembly, this assembly will NOT be compliant.
In our builds when specifying <CLSCompliant>false</CLSCompliant> in the csproj, this attribute will be fully removed, and the assembly will not tagged at all which make this assembly NOT compliant. But, if the csproj doesn't have <CLSCompliant>, the build manually adds <CLSCompliant>true</CLSCompliant> and making the assembly compliant. This means the change here is making the library CLS compliant. I confirmed that by checking the produced assemblies and ensuring the attribute is showing there with true value.

Are you aware of any scenario where the experience gets worse? I can't think of anything offhand.

I am not either, adding @ericstj @ViktorHofer if they can think in any broken scenario after this change. The opposite direction is the breaking one, I guess.

@ericstj

ericstj commented Jul 19, 2023

Copy link
Copy Markdown
Member

I'm not aware of anything breaking by adding CLSCompliant(true).

@noahfalk noahfalk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

@tarekgh tarekgh merged commit a524267 into dotnet:main Jul 20, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Aug 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants