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
updated to 13.0.2, added pragmas #1902
Conversation
@@ -77,7 +80,12 @@ public override string Get(char[] key, int start, int length) | |||
hashCode -= hashCode >> 5; | |||
|
|||
// make sure index is evaluated before accessing _entries, otherwise potential race condition causing IndexOutOfRangeException | |||
var index = hashCode & _mask; | |||
#if NET20 || NET35 || NET40 |
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.
We don't need any of those defines
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.
Both are in the System.Threading and apply to all platforms we support.
|
||
if (CloseOutput && _writer != null) | ||
{ | ||
#if HAVE_ASYNC_DISPOABLE |
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.
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 changed it, to be consistent with the repo, but the define in M.IM.Tokens.csproj was also misspelled.
So, i changed them both.
@@ -1,5 +1,5 @@ | |||
| |||
#if (DOTNET || PORTABLE40 || PORTABLE || NET_CORE) | |||
#if (DOTNET || PORTABLE40 || PORTABLE) |
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.
We use NET_CORE in our test code, i don't think it is related to the product code, but i will add it back.
PORTABLE40 and PORTABLE are not defined in our builds, and are in many locations in the newtonsoft source.
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.
src/Microsoft.IdentityModel.Tokens/opensource/json/DefaultJsonNameTable.cs
Show resolved
Hide resolved
@@ -25,6 +25,8 @@ | |||
|
|||
using System; | |||
|
|||
#nullable disable |
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.
That code is in the 13.0.2 branch and not in our last snapshot.
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.
Reviewed all the files for the first 3rd then spot checked the rest. LGTM |
@@ -119,7 +128,7 @@ public string Add(string key) | |||
hashCode -= hashCode >> 5; | |||
for (Entry entry = _entries[hashCode & _mask]; entry != null; entry = entry.Next) | |||
{ | |||
if (entry.HashCode == hashCode && entry.Value.Equals(key)) | |||
if (entry.HashCode == hashCode && entry.Value.Equals(key, StringComparison.Ordinal)) |
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.
@@ -158,7 +167,12 @@ private void Grow() | |||
} | |||
} | |||
_entries = newEntries; | |||
_mask = newMask; | |||
|
|||
#if NET20 || NET35 || NET40 |
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.
Like read, we probably don't need this, maybe just use Volatile.Write()?
Snapped to version 13.0.2.
The majority of the changes we #nullable enable as 13.0.2 uses nullable refs.
The other changes were suppressing warnings.
The changes in the source is the result of internal changes.
To see what those are you can compare these changes to the newtonsoft repo.