-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Add AssemblyExtensions.GetApplyUpdateCapabilities method #51954
Conversation
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
/cc @mikem8361 @tmat |
0493252
to
301ca01
Compare
|
||
var result = mi.Invoke(null, null); | ||
|
||
Assert.NotNull(result); |
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.
It's bit odd to check for notnull if the return value is not declared as nullable
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.
Nullable annotations are just a suggestions. They do not guarantee any behavior. We have similar checks in many other tests.
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.
LGTM
Hello @lambdageek! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
@@ -62,4 +62,13 @@ | |||
<method signature="System.Void Initialize()" /> | |||
</type> | |||
</assembly> | |||
|
|||
<!-- methods used by hot reload. | |||
TODO: once there's a feature flag, add it to the linker descriptor |
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.
Can we use the Debugger.IsSupported
feature switch for now? That's what we did for MetadataUpdateHandlerAttribute
.
runtime/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.LinkAttributes.Shared.xml
Lines 33 to 36 in 636988a
<!-- Hot reload attributes--> | |
<type fullname="System.Reflection.Metadata.MetadataUpdateHandlerAttribute"> | |
<attribute internal="RemoveAttributeInstances" /> | |
</type> |
I can put a PR to fix this.
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.
Yes, that makes sense to me.
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 opened #51994
Implement a private API that returns the hot reload capabilities of the current runtime.
The corresponding Roslyn API work is dotnet/roslyn#52566
Resolves #50111