Specify units in After(int delayInMilliseconds, int pollingInterval)
#4704
Conversation
Although milliseconds is the natural unit for an `int` delay, specifying can still be helpful just to be sure.
| @@ -304,7 +304,7 @@ public DelayedConstraint.WithRawDelayInterval After(int delay) | |||
| /// and polling interval. | |||
| /// </summary> | |||
| /// <param name="delayInMilliseconds">The delay in milliseconds.</param> | |||
| /// <param name="pollingInterval">The interval at which to test the constraint.</param> | |||
| /// <param name="pollingInterval">The interval at which to test the constraint, in milliseconds.</param> | |||
There was a problem hiding this comment.
It would be more consistent to rename the parameter into pollingIntervalInMilliseconds to match the delay one.
There was a problem hiding this comment.
If that’s an acceptable break, I’ll make that change next time I’m in front of a keyboard
There was a problem hiding this comment.
Previously I would have said yes, but thinking off it, if someone did .After(100, pollingInterval: 10) it would be a breaking change. So therefore all we can change is the comments.
There was a problem hiding this comment.
If it’s valuable enough, this method can be obsoleted in favor of the enhanced syntax .After(100).Milliseconds.PollEvery(10).Milliseconds. It would be the perfect scenario for an analyzer and fixer.
There was a problem hiding this comment.
Looks great to me, thanks for your contribution @RenderMichael !
I'll leave this unmerged for now to allow @manfred-brands the opportunity to review too as they had been involved in earlier discussions.
Although milliseconds is the natural unit for an
intdelay, specifying can still be helpful just to be sure.