Skip to content

Enable nullable warnings in a number of C# files - #4252

Open
Nateowami wants to merge 1 commit into
masterfrom
fix/enable-nullable-warnings
Open

Nateowami wants to merge 1 commit into
masterfrom
fix/enable-nullable-warnings

Conversation

@Nateowami

@Nateowami Nateowami commented Nov 7, 2026 •

Copy link
Copy Markdown
Collaborator

This removes #nullable disable warnings where it is possible to do so without breaking things, or where only very minor changes are needed.

It introduces non-null assertions and marks some things as optional that were previously not marked as optional. However, in general this is merely making the code more honest about what is optional and when it is doing something the compiler can't prove is safe, NOT making something unsafe that was safe before.

The main benefit isn't to the code that is updated here, but the fact that lots of future code will be more strictly checked for type safety.

This change is Reviewable

@codecov

codecov Bot commented Nov 7, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 65.90909% with 45 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.31%. Comparing base (c429a36) to head (3d91a8c).
⚠️ Report is 2 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...ure/Services/MachineServiceCollectionExtensions.cs 0.00% 5 Missing ⚠️
src/SIL.XForge.Scripture/Startup.cs 28.57% 5 Missing ⚠️
...XForge.Scripture/Controllers/ParatextController.cs 63.63% 3 Missing and 1 partial ⚠️
...IL.XForge.Scripture/Services/DotNetCoreRegistry.cs 20.00% 4 Missing ⚠️
src/SIL.XForge/Models/InterfaceLanguage.cs 0.00% 4 Missing ⚠️
...ge/Services/JsonRpcApplicationBuilderExtensions.cs 0.00% 3 Missing ⚠️
src/SIL.XForge.Scripture/Migrator.cs 0.00% 2 Missing ⚠️
...IL.XForge.Scripture/Models/EditorTabPersistData.cs 0.00% 2 Missing ⚠️
...c/SIL.XForge.Scripture/Models/OnboardingRequest.cs 33.33% 2 Missing ⚠️
...L.XForge.Scripture/Services/SFScrTextCollection.cs 0.00% 2 Missing ⚠️
... and 12 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4252      +/-   ##
==========================================
+ Coverage   81.30%   81.31%   +0.01%     
==========================================
  Files         671      671              
  Lines       43020    43086      +66     
  Branches     7138     7119      -19     
==========================================
+ Hits        34976    35035      +59     
- Misses       6877     6899      +22     
+ Partials     1167     1152      -15     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@Nateowami
Nateowami deployed to screenshot_diff November 7, 2026 03:44 — with GitHub Actions Active
@Nateowami
Nateowami force-pushed the fix/enable-nullable-warnings branch from 566314e to 3d91a8c Compare November 7, 2026 03:48
@Nateowami
Nateowami marked this pull request as ready for review November 7, 2026 03:48
@Nateowami Nateowami added the ai-review-author-sign-off PR author has looked at AI review of change and determined no findings warrant further changes label Nov 7, 2026
@Nateowami
Nateowami deployed to screenshot_diff November 7, 2026 03:56 — with GitHub Actions Active

This branch was successfully deployed

1 active deployment
screenshot_diff — 3d91a8ca Deployed Nov 7, 2026 by Nateowami via Compare Screenshots #1305
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review-author-sign-off PR author has looked at AI review of change and determined no findings warrant further changes testing not required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant