bito-code-review[bot] commented on code in PR #40925:
URL: https://github.com/apache/superset/pull/40925#discussion_r3387799711
##########
superset-frontend/packages/superset-ui-core/test/number-format/factories/createDurationFormatter.test.ts:
##########
@@ -35,7 +35,7 @@ test('format milliseconds in human readable format with
default options', () =>
});
test('format seconds in human readable format with default options', () => {
const formatter = createDurationFormatter({ multiplier: 1000 });
- expect(formatter(-0.5)).toBe('-0s');
+ expect(formatter(-0.5)).toBe('0s');
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Inconsistent negative value formatting</b></div>
<div id="fix">
This change masks an inconsistency: line 28 shows `formatter(-1000)` formats
to `'-1s'` preserving the negative sign, but `-0.5` with multiplier=1000 now
formats to `'0s'` losing the sign. The original `'-0s'` expectation was
semantically more consistent — it signaled that a negative value exists even if
it rounds to zero display. Consider whether this behavior change is intentional
or if the `isAllUnitsZero` check in `createDurationFormatter.ts` should
explicitly handle JavaScript's `-0` special value.
</div>
</div>
<small><i>Code Review Run #f9fb77</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
