-
Notifications
You must be signed in to change notification settings - Fork 416
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feature(monitor): add Jobs tab in tui mode #8601
feature(monitor): add Jobs tab in tui mode #8601
Conversation
47d7e3a
to
6cef4ba
Compare
84a63db
to
954c540
Compare
56c0616
to
177b444
Compare
Marking this as draft for now, as I still need to improve some things.
|
83bc90e
to
53625ce
Compare
Signed-off-by: Ali Caglayan <[email protected]>
53625ce
to
30a2179
Compare
@@ -0,0 +1,11 @@ | |||
module type S = sig |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to get rid of this module?
~f:(fun (_ : Job.Id.t) displayed_job -> displayed_job.row, displayed_job) | ||
|> Int.Map.of_list_exn | ||
in | ||
(* CR-someday amokhov: We should highlight jobs that were running for longer than 5s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these comments are copy-pasted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes but they are relevant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are duplicated because we have the same issue with both jobs display backends. The coloring issues will have to be solved independently however the description shortening and anonymous action filtering could share a solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do coloring issues have to be solved independently? We can use Pp.t
to set the colors in both.
} | ||
end | ||
|
||
type t = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about the names Tab
and Tab.Bar
instead?
} | ||
end | ||
|
||
module Display (_ : Jobs_display_intf.S with type state = State.t) : sig |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can easily get rid of this functor and just have a function that takes a record.
FTR, this is waiting on #8619. We decided that we want this feature in and out of TUI. |
When
dune monitor
is run with--display tui
, we now get an extra tab that can be clicked. This displays the currently running jobs and recently finished ones.dune monitor
.