Added byte rate calculation functons, closes #296 #314
Conversation
| { | ||
| public class ByteRate | ||
| { | ||
| public ByteSize size { get; private set;} |
There was a problem hiding this comment.
Please change the property names to CamelCase.
|
The CI build is failing because of missing XML comments on Also please review contribution guidelines
|
| var rate = size.Per(measurementInterval); | ||
| var text = rate.Humanize(displayInterval); | ||
|
|
||
| text = size.Per(measurementInterval).Humanize(displayInterval); |
There was a problem hiding this comment.
I am not sure why you're setting text, size and measurementInterval variables here again!! I am removing them. Please remove them.
There was a problem hiding this comment.
Shoot, I was playing with the API and totally forgot to remove it when I was done!
|
You have a failing test. Please run all the tests and fix the ApiApprover test. |
|
The ApiApprover test doesn't run on my machine, but I've taken a crack at fixing it. I believe I've addressed the other issues noted above. E: Looks like I failed to fix the ApiApprover test. It doesn't look like I can download the generated text file that's being tested against from TC, is there any chance you could gist it? |
| @@ -8,6 +8,7 @@ | |||
| - [#307](https://github.com/MehdiK/Humanizer/pull/307): Added support to string.FormatWith for the explicit culture parameter | |||
| - [#312](https://github.com/MehdiK/Humanizer/pull/312): Added Turkish ToWord, ToOrdinalWord and Ordinalize implementation | |||
| - [#173](https://github.com/MehdiK/Humanizer/pull/173): Added support for Window Phone 8.1 | |||
| - [#314](https://github.com/MehdiK/Humanizer/pull/314): Added ByteRate class and supporting members to facilitate calculation of byte transfer rates | |||
There was a problem hiding this comment.
This line should move to the development section.
@MehdiK why do you release stuff while other are still developing 😠 :ironic:
When the test fails you should see a diff tool popping up showing the expected text. Not sure why it's not working on your machine! Can you run it using a different runner? Please rebase your work on top of the upstream & move the release notes up to the dev section. Thanks. |
|
Finally I've found some time to fix the ApiApprover test. |
|
Thanks Max! My apologies for not fixing the test myself. I've tried it on three different pc's since the dialog here and have yet to see it run correctly. On Thu, Aug 7, 2014 at 5:12 PM, Max Malook [email protected]
|
|
@mexx thanks for fixing, rebase and pushing this up. You're awesome. Just one small thing: please use |
|
This is now released to NuGet as v1.29.0. Thanks for the great addition and your collaboration. |
API is
var rateString = bytesize.Per(timespan).ToRatePerSecond();