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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.Net: #5908 conditionalize dependencies #6479

Conversation

thompson-tomo
Copy link

Motivation and Context

This reduces the dependency graph by making STJ only included on net standard

Description

This simply makes dependencies which are available as part of the framework conditional so that they are not being un-necessarily included

Contribution Checklist

Closes #5908

@thompson-tomo thompson-tomo requested a review from a team as a code owner May 31, 2024 23:51
@markwallace-microsoft markwallace-microsoft added .NET Issue or Pull requests regarding .NET code kernel Issues or pull requests impacting the core kernel kernel.core memory labels May 31, 2024
@github-actions github-actions bot changed the title #5908 conditionalize dependencies .Net: #5908 conditionalize dependencies May 31, 2024
@thompson-tomo thompson-tomo force-pushed the chore/#5908_ConditionalPackages branch from f557646 to 01621bc Compare June 18, 2024 11:39
@WeihanLi
Copy link
Contributor

maybe also Microsoft.BCL.* packages

@thompson-tomo thompson-tomo force-pushed the chore/#5908_ConditionalPackages branch from 01621bc to ca081c0 Compare June 18, 2024 12:39
@thompson-tomo thompson-tomo force-pushed the chore/#5908_ConditionalPackages branch 2 times, most recently from eed8bb5 to 7a5a192 Compare June 19, 2024 13:00
@thompson-tomo thompson-tomo force-pushed the chore/#5908_ConditionalPackages branch from 7a5a192 to 5daca72 Compare June 19, 2024 13:38
@thompson-tomo thompson-tomo force-pushed the chore/#5908_ConditionalPackages branch 2 times, most recently from 745389d to e70097c Compare June 20, 2024 10:04
@thompson-tomo thompson-tomo force-pushed the chore/#5908_ConditionalPackages branch from e70097c to 35a8875 Compare June 29, 2024 23:56
@thompson-tomo thompson-tomo force-pushed the chore/#5908_ConditionalPackages branch 2 times, most recently from 9398ac6 to b370870 Compare July 16, 2024 03:31
@thompson-tomo thompson-tomo force-pushed the chore/#5908_ConditionalPackages branch from b370870 to 1edecc6 Compare July 18, 2024 22:07
@markwallace-microsoft markwallace-microsoft added this pull request to the merge queue Jul 23, 2024
@markwallace-microsoft markwallace-microsoft removed this pull request from the merge queue due to a manual request Jul 23, 2024
@@ -18,10 +18,11 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.Bcl.AsyncInterfaces" />
<PackageReference Include="Microsoft.Bcl.HashCode" />

Choose a reason for hiding this comment

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

Should this be in the netstandard2.0 item group?

Copy link
Author

Choose a reason for hiding this comment

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

U are right will make that additional cleanup tomorrow morning my time.

Copy link
Author

Choose a reason for hiding this comment

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

@markwallace-microsoft it appears that Microsoft.BCL.HashCode is not needed at all & Microsoft.Bcl.AsyncInterfaces is being imported as a transitive dependency. Not sure if the abstractions explicitly need this dependency.

@thompson-tomo thompson-tomo force-pushed the chore/#5908_ConditionalPackages branch from 9253869 to 2ab2a03 Compare July 24, 2024 00:30
@markwallace-microsoft
Copy link
Member

markwallace-microsoft commented Aug 14, 2024

@dmytrostruk is going to continue this work, tracking with #8109 . We want to make sure we have the correct test coverage before merging this.

@markwallace-microsoft markwallace-microsoft self-assigned this Sep 20, 2024
@markwallace-microsoft
Copy link
Member

@dmytrostruk is going to continue this work, tracking with #8109., this PR will be updated as part of this task

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kernel.core kernel Issues or pull requests impacting the core kernel memory .NET Issue or Pull requests regarding .NET code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

.Net: Leverage framework provided System.Text.Json on net 8
5 participants