Repository navigation
Carry a duration into the next unit instead of printing 60s, 60m and 1h60m - #763
Merged
Merged
Conversation
…1h60m `formatMs` chose its unit from the raw milliseconds and rounded inside that unit, so a value just under a boundary printed a count its own unit cannot hold: 59.6 s -> `60s`, 59m59.999s -> `60m`, 1h59m50s -> `1h60m`. The run report prints this as a wait, so it reached a person. Round once and carry: seconds below a minute, then whole minutes, then `h` plus two-digit minutes. Boundary cases pin it. Fixes #761
`arch-docs` asks for the subsystem doc when an owned file changes; the digest row now says a duration carries into the next unit, so the text never prints `60s`, `60m` or `1h60m`. Fixes #761
Collaborator
Author
|
Verdict: PASS at 3394f6c Ran (head 3394f6c tarball over main 0105eb2; Node 22.23.3; DSH_HARNESS=/opt/flybot, TZ=UTC):
What I checked:
Not run: the Non-blocking: none found. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Written-by: Dev
Fixes #761.
Tip SHA: 3394f6c
Reproduce
formatMsinplugins/timeline/src/digest.jschose its unit from the raw milliseconds and then rounded inside that unit, so a value just under a boundary printed a count its own unit cannot hold:The run report prints it (
- longest wait: <from> → <to>, <formatMs>), so a person pasting the report into a standup reads "1h60m". Reviewer found it while judging #735; the rounding predates that PR.What changed
plugins/timeline/src/digest.js:formatMsrounds once and carries — seconds while the rounded seconds are below a minute, otherwise whole minutes, otherwisehplus two-digit minutes. No other function, endpoint or panel code changed.plugins/timeline/test/digest.test.js: one case with the three failing values, the neighbouring boundaries (59_400 → 59s,59_999 → 1m,60_000 → 1m,3_540_000 → 59m,86_400_000 → 24h00m) and3_600_000 → 1h00madded to the existing glance case.docs/subsystems/timeline.md: thedigest.jsrow now says a duration carries (1m,2h00m, never60s/1h60m). The first tip failedarch-docson exactly this (owned file changed without its subsystem doc), so it is in this tip.changelog.d/761-format-ms-carry.fixed.md: the fragment.What I ran
From a full checkout at main
0105eb25with this branch's files,DSH_HARNESS=/opt/flybot:The 116 pre-existing plugin cases stay green; the report's own
12m/1h05massertions are unchanged.Red without the source change
Restore main's
formatMs(unit chosen from the raw value, rounded inside it) inplugins/timeline/src/digest.js:The case names each value it printed (
60s,1h00mexpected but60mprinted,2h00mexpected but1h60mprinted). Restoring the new body returns 9/9 and the whole plugin 125/125.Not run
npm run verifyand the core suite: this box has nocore/sources, so only the plugin suite ran here; the core suite is unaffected by this change and is left to CI.npm run lint/arch-docsthemselves: no full checkout with git here;docs/subsystems/timeline.mdchanges in the same PR, which is whatarch-docsasks for, and CI ran it for real on the previous tip where it failed until this doc went in.panel.test.jscases.Risk
T2 by the repository's own rules (
plugins/timeline/**), so it needs a PASS at the tip. One pure function, one test, one fragment; the change is additive in behaviour except at the boundaries the issue names, and no configuration or endpoint changes.