Replace Contract.Requires with Debug.Assert #11600
Conversation
| @@ -535,7 +535,7 @@ private static void CheckBaseAddress(Uri baseAddress, string parameterName) | |||
|
|
|||
| private void SetTimeout(CancellationTokenSource cancellationTokenSource) | |||
| { | |||
| Contract.Requires(cancellationTokenSource != null); | |||
| Debug.Assert(cancellationTokenSource != null); | |||
There was a problem hiding this comment.
Can we remove the using imports for System.Diagnostics.Contracts in some of these files where Contract.Assert was the only use of contracts?
There was a problem hiding this comment.
In my first pass I removed the using and project.json entries for them all; but it got a bit upset... As there were other uses.
Will make another pass.
There was a problem hiding this comment.
Yeah, we can't for all of them, as there are other uses of Contract still, like Contract.Ensure, Contract.End*, etc.
|
A few comments, but otherwise LGTM. I admit I'm surprised to see the CI legs passing; I was expecting at least a few of the contracts would have been stale enough to cause a failure. |
benaadams
force-pushed
the
contracts
branch
2 times, most recently
from
f77b655
to
fcd2f83
Compare
|
Removed the usings; tried to remove the unneeded references but its quite tricky - might need an automated process. |
|
Thanks, Ben. |
| @@ -745,7 +745,7 @@ public override int Read([In, Out] Byte[] array, Int32 offset, Int32 count) | |||
| Task semaphoreLockTask, bool useApmPattern) { | |||
|
|
|||
| // Same conditions validated with exceptions in ReadAsync: | |||
| // (These should be Contract.Requires(..) but that method had some issues in async methods; using Assert(..) for now.) | |||
| // (These should be Debug.Assert(..) but that method had some issues in async methods; using Assert(..) for now.) | |||
There was a problem hiding this comment.
This comment no longer makes sense. It can just be deleted.
|
Two remaining nits, otherwise LGTM. Thanks, @benaadams. Since CI is green, including with multiple outer loop runs, I'll merge and then fix the comments nits separately. |
* Replace Contract.Requires with Debug.Assert * Remove unnessary Diagnostics.Contracts usings Commit migrated from dotnet/corefx@3c0e848
See: #11596