Skip to content

Conversation

@stefannikolei
Copy link
Contributor

Contains work for SixLabors/ImageSharp#2236

@codecov
Copy link

codecov bot commented Sep 20, 2022

Codecov Report

Merging #34 (28bfbea) into main (bf398a9) will decrease coverage by 0%.
The diff coverage is 14%.

@@        Coverage Diff         @@
##           main   #34   +/-   ##
==================================
- Coverage    20%   19%   -1%     
==================================
  Files         4     4           
  Lines       352   346    -6     
  Branches     88    85    -3     
==================================
- Hits         71    68    -3     
+ Misses      281   278    -3     
Flag Coverage Δ
unittests 19% <14%> (-1%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/SharedInfrastructure/DebugGuard.cs 43% <0%> (+1%) ⬆️
src/SharedInfrastructure/ThrowHelper.cs 92% <ø> (-1%) ⬇️
src/SharedInfrastructure/Guard.cs 44% <100%> (-2%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@stefannikolei
Copy link
Contributor Author

@JimBobSquarePants I think this needs to be merged first. After this changes are in the repo my PR with nullable changes should hopefully compile. (IT works on my machine ;) )

* Added CallerArgumentExpression to auto infer paramName
* Added NotNull Attribute to tell the compiler that the parameter is not null when the method return
* Removed unused method
* Add nullable to the test project
@stefannikolei
Copy link
Contributor Author

@JimBobSquarePants This could be the next step for NRT.
When this annotations make it to the main Repo, some NRT Errors will disappear.

But because of the Conditional Attribute the ArgumentNullException will not be thrown in Release builds. Is this intended?

@JimBobSquarePants
Copy link
Member

I’m not sure I understand your question? We have to guard classes. Guard and DebugGuard the latter designed for debugging only.

@stefannikolei
Copy link
Contributor Author

stefannikolei commented Dec 18, 2022

The DebugGuard will only execute while Debugging. So in release mode the method will not be executed and no ArgumentNullException will be thrown. Perhaps a NullReferenceException when the value is used which was passed to the DebugGuard.

Perhaps after finishing NRT the DebugGuard.NotNull can be removed because everything is "hopefully" save ;)

@JimBobSquarePants
Copy link
Member

JimBobSquarePants commented Dec 18, 2022

That’s by design. We don’t want DebugGuard running in release mode.

Yeah agreed. It’s likely with the proper nullable fixes we won’t even require it.

@JimBobSquarePants JimBobSquarePants merged commit 9a82679 into SixLabors:main Dec 18, 2022
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.

2 participants