Set CLSCompliant attribute in System.ClientModel#58976
Conversation
github-actions
Bot
added
Azure.Core
Community Contribution
|
Thank you for your contribution @WolfgangHG! We will review the pull request and get back to you soon. |
There was a problem hiding this comment.
Pull request overview
This pull request makes System.ClientModel declare CLS compliance at the assembly level, then explicitly marks individual public APIs that use non-CLS types (e.g., sbyte, uint, ulong, ushort) as [CLSCompliant(false)] so the rest of the assembly can be treated as CLS-compliant by consumers and tooling.
Changes:
- Add
[assembly: CLSCompliant(true)]toSystem.ClientModel. - Mark specific public APIs that expose non-CLS types as
[CLSCompliant(false)](pipeline classifier factory and selectedJsonPatchoverloads).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| sdk/core/System.ClientModel/src/Properties/AssemblyInfo.cs | Declares the assembly as CLS-compliant. |
| sdk/core/System.ClientModel/src/Options/PipelineMessageClassifier.cs | Marks the ReadOnlySpan<ushort> factory overload as non-CLS-compliant. |
| sdk/core/System.ClientModel/src/ModelReaderWriter/JsonPatch.cs | Marks JsonPatch overloads that use sbyte/unsigned integer types as non-CLS-compliant. |
| /// classifier instance will classifiy as success codes.</param> | ||
| /// <returns>A <see cref="PipelineMessageClassifier"/> instance that | ||
| /// classifies the status codes provided by | ||
| /// <paramref name="successStatusCodes"/> as success codes.</returns> | ||
| [CLSCompliant(false)] |
There was a problem hiding this comment.
I agree with copilot, but this comment is not from me. Shall I fix it anyway?
| /// <summary> | ||
| /// Gets a Int8 value at the specified JSON path. | ||
| /// </summary> | ||
| /// <param name="jsonPath">The JSON path of the value to get.</param> | ||
| /// <exception cref="KeyNotFoundException">If the <paramref name="jsonPath"/> was not found.</exception> | ||
| [CLSCompliant(false)] |
There was a problem hiding this comment.
I agree with copilot, but this comment is not from me. Shall I fix it anyway?
WolfgangHG
force-pushed
the
clscompliant_systemclientmodel
branch
from
995a05b to
b789203
Compare
I started setting the CLSCompliant attribute in kiota-dotnet (microsoft/kiota-dotnet#705). This is not possible for a project that references "Azure.Core".
And "Azure.Core" cannot be made CLSCompliant, because it uses code from "System.ClientModel",which is not CLSCompliant either.
So I started setting the attribute in "System.ClientModel", which means that a bunch of methods that use e.g.
sbyteorulongmust be defined as[CLSCompliant(false)].If you accept this pull request, the next step might be to update the package reference in "Azure.Core" and to the same there?