Skip to content

Conversation

artemmufazalov
Copy link
Member

@artemmufazalov artemmufazalov commented Jul 5, 2023

Screen Shot 2023-07-05 at 17 36 50

@artemmufazalov artemmufazalov force-pushed the format-bytes branch 2 times, most recently from d97fcd0 to 8c0b073 Compare July 5, 2023 14:34
@artemmufazalov artemmufazalov marked this pull request as ready for review July 5, 2023 14:40
@artemmufazalov artemmufazalov requested a review from Raubzeug July 5, 2023 14:40
let result = (Number(value) / sizes[size].value).toFixed(precision);
result = formatNumber(result);

if (withLabel) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets move this case and case with isSpeed to another functions. Now it's pretty unclear what for this function is responsible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you suggest separating conditions? Or there shouldn't be logic with adding label in such function?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see it as two additional functions: one will give simple label, one will give /s label.

function withSpeed(val) {
return withLabel(val)
}
function withLabel(val) {
return formatBytes(val)
}

@@ -0,0 +1,31 @@
import {formatBytes} from '../formatBytes';

describe('formatBytes', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets add test-cases with invalid data.

@artemmufazalov artemmufazalov force-pushed the format-bytes branch 2 times, most recently from 51ef43c to bd822de Compare July 7, 2023 07:40
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