Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

enable nullable #1599

Merged
merged 17 commits into from
Oct 3, 2023
Merged

enable nullable #1599

merged 17 commits into from
Oct 3, 2023

Conversation

rkm
Copy link
Member

@rkm rkm commented Aug 6, 2023

Proposed Changes

Enable the C# nullable feature and fix all warnings.

Many of the ! annotations around the GlobalOptions types are awkward since they are publicly set-able and can be null depending on what's loaded from the YAML config, but it's a good start.

Types of changes

What types of changes does your code introduce? Tick all that apply.

  • Bugfix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation-Only Update (if none of the other choices apply)
    • In this case, ensure that the message of the head commit from the source branch is prefixed with [skip ci]

Checklist

By opening this PR, I confirm that I have:

  • Reviewed the contributing guidelines for this repository
  • Ensured that the PR branch is in sync with the target branch (i.e. it is automatically merge-able)
  • Updated any relevant API documentation
  • Created or updated any tests if relevant
  • Created a news file
    • NOTE: This must include any changes to any of the following files: default.yaml, any of the RabbitMQ server configurations, GlobalOptions.cs
  • Listed myself in the CONTRIBUTORS file 🚀
  • Requested a review by one of the repository maintainers

Issues

@rkm rkm force-pushed the feature/nullable branch from 8897fa5 to d013920 Compare August 6, 2023 14:22
@codecov
Copy link

codecov bot commented Aug 6, 2023

Codecov Report

Attention: 96 lines in your changes are missing coverage. Please review.

Comparison is base (e330901) 68.44% compared to head (05b4d6e) 68.56%.
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1599      +/-   ##
==========================================
+ Coverage   68.44%   68.56%   +0.11%     
==========================================
  Files         185      185              
  Lines        6510     6550      +40     
==========================================
+ Hits         4456     4491      +35     
- Misses       2054     2059       +5     
Files Coverage Δ
...Execution/DirectoryFinders/DicomDirectoryFinder.cs 80.39% <100.00%> (+2.13%) ⬆️
...essor/Options/DicomDirectoryProcessorCliOptions.cs 42.85% <100.00%> (-10.09%) ⬇️
...c/applications/Applications.DicomLoader/Program.cs 80.00% <100.00%> (ø)
...cations/Applications.DynamicRulesTester/Program.cs 100.00% <100.00%> (ø)
...ions/Applications.ExtractImages/CohortCsvParser.cs 100.00% <100.00%> (ø)
...lications.ExtractImages/ExtractImagesCliOptions.cs 30.00% <100.00%> (ø)
...ns/Applications.ExtractImages/ExtractImagesHost.cs 97.72% <100.00%> (-2.28%) ⬇️
...lications.ExtractImages/ExtractionMessageSender.cs 100.00% <100.00%> (ø)
...ons.TriggerUpdates/Execution/TriggerUpdatesHost.cs 100.00% <100.00%> (ø)
...Updates/Options/TriggerUpdatesFromMapperOptions.cs 100.00% <100.00%> (ø)
... and 123 more

... and 1 file with indirect coverage changes

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

@rkm rkm marked this pull request as ready for review August 6, 2023 14:50
@rkm rkm requested a review from jas88 August 6, 2023 14:50
@jas88
Copy link
Contributor

jas88 commented Aug 6, 2023

@rkm Probably better using ? rather than ! for a lot of those aren't we? Good idea though, I'll take a closer look when I'm officially back from AL!

@rkm
Copy link
Member Author

rkm commented Sep 20, 2023

@jas88 do you have capacity to review this? I can get someone else to review if not 😃

@jas88
Copy link
Contributor

jas88 commented Sep 20, 2023

@rkm yep - now RDMP 8.1.0 is out I'll try to do this one later today

@rkm
Copy link
Member Author

rkm commented Oct 3, 2023

force merging this without a review so as to unblock other PRs

@rkm rkm merged commit 60c4208 into master Oct 3, 2023
@rkm rkm deleted the feature/nullable branch January 18, 2024 20:37
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable nullable types
2 participants