Skip to content

Conversation

@jonpryor
Copy link
Contributor

@jonpryor jonpryor commented Nov 12, 2021

Context: 3226a4b
Context: 7068f4b

Enable C# 8 Nullable Reference Types for
Xamarin.Android.Tools.Bytecode.dll.

Fix various warnings emitted by string globalization code analyzers.

@jonpryor jonpryor requested a review from jpobst November 12, 2021 00:22
@jonpryor jonpryor force-pushed the jonp-bytecode-nrt branch 3 times, most recently from c3a8118 to b58fa1b Compare November 12, 2021 17:00
Copy link
Contributor

@jpobst jpobst left a comment

Choose a reason for hiding this comment

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

Enable code analyzers for string globalization.

I am curious about this. Nothing in this PR changes the enabled code analyzers. It looks like this fixes the issues that would be raised by CA1307 onnet6.0, which we explicitly disabled in 7068f4b because the needed overloads are not available on netstandard2.0.

@jonpryor
Copy link
Contributor Author

Nothing in this PR changes the enabled code analyzers.

Correct, but an "intermediary" version of this commit didn't build for netstandard 2.0, but only net6.0, and when $(TargetFramework)=net6.0, lots of CA1307 warnings were emitted, until I fixed them or silenced them.

Also "funny" is the difference between netstandard2.0 & net6.0 around string.IsNullOrEmpty(): net6.0 appears to have [NotNullWhen(false)], while netstandard2.0 doesn't, hence the need for things like:

if (path == null || string.IsNullOrEmpty (path))

'cause without path == null, netstandard2.0 builds would emit CA1307 warnings.

Context: 3226a4b
Context: 7068f4b

Enable [C# 8 Nullable Reference Types][0] for
`Xamarin.Android.Tools.Bytecode.dll`.

Fix various warnings emitted by string globalization code analyzers.

[0]: https://docs.microsoft.com/en-us/dotnet/csharp/nullable-references
@jonpryor jonpryor merged commit 0293360 into dotnet:main Nov 16, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Apr 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants