-
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
Api Diff between preview 7 and preview 6 #4977
Conversation
Signed off on |
adding @MichalStrehovsky for DynamicallyAccessedMemberTypes enum rename |
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.
MVC changes look good
# Microsoft.AspNetCore.Authentication | ||
|
||
``` diff | ||
namespace Microsoft.AspNetCore.Authentication { |
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.
/cc @HaoK
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.
Yep these changes look good
DynamicallyAccessedMemberTypes enum rename looks good. |
FYI @Anipik I'm no longer with the .NET team, so should be removed from the list of reviewers for API diffs. The others you currently have on the list should already be providing coverage for my areas (or reach out to @Pilchie for other area owners in ASP.NET). Looks like y'all have done a lot of cool work though! 🎉 |
can everyone tagged please review their areas ? |
System.Text and System.Threading.Tasks looks good to me. |
``` diff | ||
namespace System.Net { | ||
public class WebHeaderCollection : NameValueCollection, IEnumerable, ISerializable { | ||
- public new string this[string name] { get; set; } |
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.
This was removed from ref in PR dotnet/runtime#37949 ... we still inherit it from NameValueCollection
Approved on behalf of @dotnet/ncl
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.
Signed off on System.Runtime.Intrinsics
and System.Runtime.Intrinsics.Arm
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.
parts of AspNetCore.
# Microsoft.AspNetCore.Server.Kestrel.Core | ||
|
||
``` diff | ||
namespace Microsoft.AspNetCore.Server.Kestrel.Core { |
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.
``` diff | ||
namespace Microsoft.Net.Http.Headers { | ||
public class SetCookieHeaderValue { | ||
+ public IList<StringSegment> Extensions { get; } |
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.
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.
Also signed off on Intrinsics.
Signed off on the numerics changes (System.Math and System.Half) as well. CC. @pgovind
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.
System.Security.* and System.Formats.Asn1 LGTM.
Surprised that System.Formats.Cbor isn't in the list.
cc @carlossanlop @jozkee @dotnet/ncl @steveharter @AaronRobinsonMSFT @jkoritzinsky @tannergooding @CarolEidt @echesakovMSFT @layomia @krwq @tarekgh @bartonjs @stephentoub @tommcdon @krwq @ericstj @richlander
@mkArtakMSFT @pranavkm @halter73 @Tratcher @BrennanConroy @anurse