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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding InternalsVisibleTo target in csproj #690

Merged
merged 2 commits into from Apr 27, 2023

Conversation

shawncal
Copy link
Member

Motivation and Context

The current syntax for "InternalsVisibleTo" is ugly, opaque, and spans multiple lines. This change cleans it up and brings it into compliance with dotnet 5+ projects.

Description

Creates a build target that transforms all single line:
<InternalsVisibleTo Include="SemanticKernelConnectorsSqliteTests" PublicKey="$(StongNamePublicKey)"/>
...into the full legacy format:

<_Parameter1>%(InternalsVisibleTo.Identity), PublicKey="%(InternalsVisibleTo.PublicKey)</_Parameter1>
<_Parameter1_TypeName>System.String</_Parameter1_TypeName>

Note: the PublicKey attribute is included in preparation for strong name signing, but this change alone does not enable/require strong name signing. I'd like to remove most of the internalvisibleto attributes before making that strong name signing change, due to the waterfall of dependencies they cause.

Contribution Checklist

@github-actions github-actions bot added .NET Issue or Pull requests regarding .NET code kernel.core labels Apr 27, 2023
SergeyMenshykh
SergeyMenshykh previously approved these changes Apr 27, 2023
@lemillermicrosoft lemillermicrosoft merged commit 1af18a1 into microsoft:main Apr 27, 2023
11 checks passed
dluc pushed a commit that referenced this pull request Apr 29, 2023
### Motivation and Context
The current syntax for "InternalsVisibleTo" is ugly, opaque, and spans
multiple lines. This change cleans it up and brings it into compliance
with dotnet 5+ projects.

### Description
Creates a build target that transforms all single line:
`<InternalsVisibleTo Include="SemanticKernelConnectorsSqliteTests"
PublicKey="$(StongNamePublicKey)"/>`
...into the full legacy format:
<AssemblyAttribute
Include="System.Runtime.CompilerServices.InternalsVisibleTo">
<_Parameter1>%(InternalsVisibleTo.Identity),
PublicKey="%(InternalsVisibleTo.PublicKey)</_Parameter1>
        <_Parameter1_TypeName>System.String</_Parameter1_TypeName>
      </AssemblyAttribute>

Note: the PublicKey attribute is included in preparation for strong name
signing, but this change alone does not enable/require strong name
signing. I'd like to remove most of the internalvisibleto attributes
before making that strong name signing change, due to the waterfall of
dependencies they cause.
dehoward pushed a commit to lemillermicrosoft/semantic-kernel that referenced this pull request Jun 1, 2023
### Motivation and Context
The current syntax for "InternalsVisibleTo" is ugly, opaque, and spans
multiple lines. This change cleans it up and brings it into compliance
with dotnet 5+ projects.

### Description
Creates a build target that transforms all single line:
`<InternalsVisibleTo Include="SemanticKernelConnectorsSqliteTests"
PublicKey="$(StongNamePublicKey)"/>`
...into the full legacy format:
<AssemblyAttribute
Include="System.Runtime.CompilerServices.InternalsVisibleTo">
<_Parameter1>%(InternalsVisibleTo.Identity),
PublicKey="%(InternalsVisibleTo.PublicKey)</_Parameter1>
        <_Parameter1_TypeName>System.String</_Parameter1_TypeName>
      </AssemblyAttribute>

Note: the PublicKey attribute is included in preparation for strong name
signing, but this change alone does not enable/require strong name
signing. I'd like to remove most of the internalvisibleto attributes
before making that strong name signing change, due to the waterfall of
dependencies they cause.
@shawncal shawncal deleted the internals-visible branch July 18, 2023 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
.NET Issue or Pull requests regarding .NET code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants