Skip to content
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 JetBrains Annotations (useless from C# 8) #1544

Closed
wants to merge 1 commit into from
Closed

Conversation

YoshiRulz
Copy link
Member

@YoshiRulz YoshiRulz commented Apr 19, 2019

Includes NotNullAttribute/CanBeNullAttribute for design-time null checks by Rider, plus some other stuff. This is effectively a backport of nullable reference types from C# 8.

The package description claims "All usages of ReSharper Annotations attributes are erased from metadata by default, which means no actual binary reference to 'JetBrains.Annotations.dll' assembly is produced."

edit: consensus is that the benefits are too few, but I'm leaving this open as an extra nag to move to Framework 4.8.

@YoshiRulz YoshiRulz added the Meta Relating to code organisation or to things that aren't code label Apr 19, 2019
@vadosnaprimer

This comment has been minimized.

@zeromus
Copy link
Contributor

zeromus commented May 25, 2019

HOW would those annotations get erased from metadata? Does the nuget package add msbuild logic? Sounds rickety...

@YoshiRulz

This comment has been minimized.

@vadosnaprimer
Copy link
Contributor

vadosnaprimer commented May 25, 2019

I'm only fine with this if it can be disabled by default (or at least just locally). These links look relevant?
https://www.jetbrains.com/help/resharper/Reference_Options_Tools_Build_NuGet.html
https://stackoverflow.com/a/45445515/2792852

To explain my take, see the SO question for how much time it takes when nuget is heavily involved. And if we start one day, in a few years it will take just as much time (or more). So being able to avoid the overhead is valuable.

...except Version, which really doesn't warrant it
@YoshiRulz
Copy link
Member Author

Checkout branch jb_annot_emuhawk to see what kind of effect this has on the build duration (it's based on e60896c).

@vadosnaprimer
Copy link
Contributor

It took a few seconds to update the packages, but then everything kept building at normal speed, without nuget notifications. And it explicitly said fetching them upon build can be disabled. So until newer packages slow us down too much, it's fine.

@adelikat
Copy link
Contributor

Not a fan of this. It is SO intrusive, for a not-free IDE that few people are using

@Scepheo
Copy link
Contributor

Scepheo commented Jun 19, 2019

HOW would those annotations get erased from metadata? Does the nuget package add msbuild logic? Sounds rickety...

To answer this: https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.conditionalattribute?view=netframework-4.8

@YoshiRulz YoshiRulz changed the title Add JetBrains Annotations Add JetBrains Annotations (useless from C# 8) Aug 18, 2019
@YoshiRulz
Copy link
Member Author

First-party nullability is opt-in enabled in BizHawk.Common from 8de0355, and after #1801 will extend to more projects.
https://github.com/TASVideos/BizHawk/blob/4ed402fc59c5ebac072ce8888f62b09b50e0f085/BizHawk.Common/IImportResolver.cs#L11-L17

@YoshiRulz YoshiRulz closed this Feb 5, 2020
@YoshiRulz YoshiRulz deleted the jb_annot branch February 5, 2020 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Meta Relating to code organisation or to things that aren't code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants