Skip to content

Conversation

@DigraJatin
Copy link

Closes #4556

What's new?

  1. Updated input_timestamp() to correctly handle future timestamps.
  2. Replaced the buggy modulo logic—which produced confusing "double negative" strings like -100.-500ms ago—with absolute duration calculations.
  3. Added a contextual suffix (ago vs in future) to make the logs human-readable regardless of clock drift or event timing. (No negative sign would be printed for future timestamps).

Past values will be printed like -
500 ms ago
Future values will be printed like -
100 ms in future

How to test

Added unit tests for the function, run ./bin/mir_unit_tests --gtest_filter=TimestampTest.* and verify.

Checklist

  • [ ✅] Tests added and pass
  • [ ✅] Adequate documentation added

@DigraJatin DigraJatin requested a review from a team as a code owner January 2, 2026 16:35
Copy link
Contributor

@tarek-y-ismail tarek-y-ismail left a comment

Choose a reason for hiding this comment

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

Looks good. One small nit...

Copy link
Contributor

@AlanGriffiths AlanGriffiths left a comment

Choose a reason for hiding this comment

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

Some tweaks and some non-blocking comments you can chose to ignore

Copy link
Contributor

@AlanGriffiths AlanGriffiths left a comment

Choose a reason for hiding this comment

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

Nice, thanks!

Copy link
Contributor

@tarek-y-ismail tarek-y-ismail left a comment

Choose a reason for hiding this comment

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

CI is failing. The following suggestion might help you fix it.

auto-merge was automatically disabled January 6, 2026 20:59

Head branch was pushed to by a user without write access

Copy link
Contributor

@tarek-y-ismail tarek-y-ismail left a comment

Choose a reason for hiding this comment

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

Looking good! Now let's see if CI is happy.

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.

Fix logging snprintf with negative time values

3 participants