Skip to content

Conversation

@reddevilmidzy
Copy link
Member

close: #606

@reddevilmidzy reddevilmidzy force-pushed the duration branch 2 times, most recently from 5766fd7 to c6beb12 Compare January 19, 2026 01:22
@Kobzol
Copy link
Member

Kobzol commented Jan 19, 2026

Hi, thanks for the PR!

What do you think about having the duration be a separate row? I think that it would be a bit easier to parse visually. Also, I'd use shortened versions of the time units (h/m/s), for the same reason.

image image image

(I also wrapped the duration in backticks)

@reddevilmidzy
Copy link
Member Author

Thank you for the example! The hh mm ss format, like the last picture, looks neat and nice. I'll update it

@Kobzol
Copy link
Member

Kobzol commented Jan 19, 2026

Could you please add (or modify an existing) test that has a different duration than just 1h? 😆 To test the logic around minutes and seconds.

You can use

let w1 = ctx.try_workflow();
ctx.modify_workflow(w1, |w| w.set_duration(Duration::from_secs(100)));

to change it.

@Kobzol
Copy link
Member

Kobzol commented Jan 19, 2026

I'm not sure if using run_test multiple times in the same test is OK, the DB state will be preserved between them. I'd split those into three separate tests.

@reddevilmidzy
Copy link
Member Author

Yes, I think that's why the test is failing😅

Copy link
Member

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

Thank you! :)

@Kobzol Kobzol added this pull request to the merge queue Jan 19, 2026
Merged via the queue into rust-lang:main with commit 78a0112 Jan 19, 2026
3 checks passed
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.

feature: write run duration into auto_build_succeeded_comment

2 participants