-
-
Notifications
You must be signed in to change notification settings - Fork 407
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
Addresses #4450 (Fix issues with Backport.System.Threading.Lock package) #4473
base: main
Are you sure you want to change the base?
Conversation
<PackageReference Include="Backport.System.Threading.Lock" Version="3.1.0" /> | ||
<PackageReference Include="Backport.System.Threading.Lock" Version="3.1.4" /> | ||
<Using Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net9.0'))" Alias="Lock" Include="System.Threading.Lock" /> | ||
<Using Condition="!$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net9.0'))" Alias="Lock" Include="Backport.System.Threading.Lock" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this point, wouldn't it make more sense to make the Lock
alias simply refer to System.Object
? This is assuming that the only way the Lock
type is currently being used is through the lock (...)
syntax and not through any method calls. That way the external dependency can be dropped in its entirety.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would not allow the new locking API though. If an external dependency is an issue it can be used as the source generator instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming the primary usage is the lock (<some Lock instance>)
syntax, if you conditionally make the Lock
alias target either System.Threading.Lock
or System.Object
, you still get the performance benefits on .NET 9 for having System.Threading.Lock
and you don't really have to think about it much more than that when writing code. Sure you can't use the new API, but I don't think many projects are actually using that to begin with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my other comment regarding usage of the Backport
package - it is only used in a couple projects, and so no other projects (including the generators) should reference the package at all right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this change mean that when people create a .NET 4.8 WinForms project, they won't need to add their own reference to the BackPort
package?
I found that I had to add this superfluous reference when updating the samples.
For example, WinSortFilter
Addresses #4450