Skip to content

Conversation

@williamjallen
Copy link
Collaborator

Dates were left undone when the new filters interface was initially introduced. This PR fills in the gaps. More work is needed to implement boolean types.

image

Copy link
Member

@josephsnyder josephsnyder left a comment

Choose a reason for hiding this comment

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

The change is good and functions well. LGTM!
I have some questions about usage in general for future updates:

  1. I don't see a way to take the /builds/###/commands and find the start time of any of each command. Combining this with the usage of the current datetime as the default value, it seems like a lot of boxes to change just to get to useful information
  2. For sufficiently small projects, like our test example which seems to run in 541ms, a second granularity isn't sufficient. If I wanted to show everything that ran before a particular command's start time, it would be difficult. I will admit that this seems like a fairly niche corner case.

@williamjallen
Copy link
Collaborator Author

  1. I don't see a way to take the /builds/###/commands and find the start time of any of each command. Combining this with the usage of the current datetime as the default value, it seems like a lot of boxes to change just to get to useful information

I plan to add a few more things to the tooltip when you hover over commands, including the command start time and measurement information, so let's defer this to a future PR.

  1. For sufficiently small projects, like our test example which seems to run in 541ms, a second granularity isn't sufficient. If I wanted to show everything that ran before a particular command's start time, it would be difficult. I will admit that this seems like a fairly niche corner case.

This is something I went back and forth on quite a bit. A dropdown is probably not the best way to specify milliseconds, but it looks weird to have a bunch of dropdowns and then a numeric entry box. I also wasn't convinced that anyone would want to filter by ms, since most events across the site happen on second timescale at most. The exception is build commands which happen on ms scale, but you can also zoom in on the chart so I'm not sure anyone will actually want to filter at ms granularity. Thoughts? If you think it's important to have, I'd be happy to add it.

@josephsnyder
Copy link
Member

I plan to add a few more things to the tooltip when you hover over commands, including the command start time and measurement information, so let's defer this to a future PR.

That's what I was hoping for. Thanks!

The exception is build commands which happen on ms scale, but you can also zoom in on the chart so I'm not sure anyone will actually want to filter at ms granularity. Thoughts? If you think it's important to have, I'd be happy to add it.

I'm not convinced its needed at the moment. I see more utility in a 3 hour build being able to see what commands before a particular second than a half second build doing the same at the millisecond scale. But someone might. Again, future work.

Thanks for the clarification!

@josephsnyder josephsnyder added this pull request to the merge queue Jan 5, 2026
Merged via the queue into Kitware:master with commit 19e970f Jan 5, 2026
13 of 14 checks passed
@williamjallen williamjallen deleted the date-filters branch January 5, 2026 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants