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

Set project as nullable #3042

Merged
merged 11 commits into from
Dec 31, 2023
Merged

Set project as nullable #3042

merged 11 commits into from
Dec 31, 2023

Conversation

shargon
Copy link
Member

@shargon shargon commented Dec 29, 2023

Note: Two bugs in cli was also fixed during the nullable checks

@@ -257,7 +257,7 @@ public void OnShowContractCommand(string nameOrHash)
ConsoleHelper.Info("", " Compiler: ", $"{contract.Nef.Compiler}");
ConsoleHelper.Info("", " SourceCode: ", $"{contract.Nef.Source}");
ConsoleHelper.Info("", " Trusts: ", $"[{string.Join(", ", contract.Manifest.Trusts.Select(s => s.ToJson()?.GetString()))}]");
if (contract.Manifest.Extra is null)
if (contract.Manifest.Extra is not null)
Copy link
Member Author

Choose a reason for hiding this comment

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

This was a bug

Copy link
Member

Choose a reason for hiding this comment

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

someone must of changed the code, i don't write code like that.

@Jim8y
Copy link
Contributor

Jim8y commented Dec 29, 2023

may you also set all project as nullable?

Jim8y
Jim8y previously approved these changes Dec 29, 2023
@Jim8y
Copy link
Contributor

Jim8y commented Dec 29, 2023

got warnings for Unchanged files with check annotations Beta

@shargon
Copy link
Member Author

shargon commented Dec 29, 2023

may you also set all project as nullable?

Yes, I think that we can set all of them, but we can wait with neo, or make a different PR

{
if (NoWallet()) return;
Copy link
Member Author

Choose a reason for hiding this comment

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

This was another bug, we can't cancel without a wallet

@shargon
Copy link
Member Author

shargon commented Dec 29, 2023

@Jim8y we can do Neo, and Neo.Gui in another PR

@shargon shargon requested a review from Jim8y December 29, 2023 12:28
@@ -45,7 +44,7 @@ private void OnTransferCommand(UInt160 tokenHash, UInt160 to, decimal amount, UI
Transaction tx;
try
{
tx = CurrentWallet.MakeTransaction(snapshot, new[]
tx = CurrentWallet!.MakeTransaction(snapshot, new[]
Copy link
Contributor

Choose a reason for hiding this comment

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

With nullable enabled, NoWallet would be unnecessary.

Copy link
Member

@cschuchardt88 cschuchardt88 Dec 29, 2023

Choose a reason for hiding this comment

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

You shouldn't do ! unless 100% sure its not going to be null. Set tx to nullable like Transaction? tx;. then check for null. That's the point of nullable. If you don't want it to be nullable than check return for null, and you dont have to use !. Also another point of nullable is for you to be checking for null.

Copy link
Member

Choose a reason for hiding this comment

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

Also the reason why you are getting the warning for nullable is because your not doing null checking. For example if (tx == null).

Copy link
Member Author

Choose a reason for hiding this comment

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

With noWallet you can use !

Copy link
Member Author

Choose a reason for hiding this comment

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

With nullable enabled, NoWallet would be unnecessary.

How?

@cschuchardt88
Copy link
Member

Nullable isn't going to fix your problems. You know how much you are going to have to change? You have to make the decision whether something should be null or not. Try using Nullable class 1st.

https://learn.microsoft.com/en-us/dotnet/api/system.nullable?view=net-7.0

@shargon shargon merged commit 447b149 into master Dec 31, 2023
2 checks passed
@shargon shargon deleted the nullable branch December 31, 2023 09:29
Jim8y added a commit to Jim8y/neo that referenced this pull request Dec 31, 2023
* master: (30 commits)
  Set project as nullable (neo-project#3042)
  Fix: fix equal (neo-project#3028)
  Added README to packages (neo-project#3026)
  Nuget MyGet Fix (neo-project#3031)
  Add: print out the stack (neo-project#3033)
  fixed myget (neo-project#3029)
  Fixed MyGet Workflow (neo-project#3027)
  Package icons - hotfix (neo-project#3022)
  Nuget Package Icon & Symbols (neo-project#3020)
  Fix warning (neo-project#3021)
  Neo-node Migration (neo-project#2990)
  Remove unnecessary default seedlist (neo-project#2980)
  Fix Neo VM target frameworks (neo-project#2989)
  Update Neo.VM location in README.md (neo-project#2988)
  Migrating Neo VM (neo-project#2970)
  3.6.2 (neo-project#2962)
  fix ut (neo-project#2959)
  Validate serialization during Contract deploy and Update (neo-project#2948)
  code optimization (neo-project#2958)
  check null scriptcontainer (neo-project#2953)
  ...
Jim8y added a commit to Jim8y/neo that referenced this pull request Jan 10, 2024
* master:
  Fixed asp.net core project (neo-project#3067)
  Updated BLS12_381 (neo-project#3074)
  avoid nonsense exception messages. (neo-project#3063)
  Removed `MyGet` (neo-project#3071)
  Updated unit-test (neo-project#3073)
  add hash verification for OnImport (neo-project#3070)
  Make public ReadUserInput (neo-project#3068)
  Removed asp.net core (neo-project#3065)
  Enforce Line Endings in `.editorconfig` (neo-project#3060)
  Remove some warnings (neo-project#3057)
  Fixed workflow  timeout-minutes (neo-project#3048)
  Fix: specify the error message (neo-project#3030)
  Removes `WebSocket`s from the network layer (neo-project#3039)
  set timeout for tests (neo-project#3046)
  Fix: Editconfig (neo-project#3023)
  Set project as nullable (neo-project#3042)
  Fix: fix equal (neo-project#3028)

# Conflicts:
#	src/Neo.CLI/CLI/MainService.cs
#	src/Neo.CLI/Settings.cs
#	src/Neo/ProtocolSettings.cs
@roman-khimov roman-khimov added this to the v3.7.0 milestone Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants