Skip to content
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

Display LV event duration #44

Merged
merged 5 commits into from
Jan 19, 2022
Merged

Display LV event duration #44

merged 5 commits into from
Jan 19, 2022

Conversation

leandrocp
Copy link
Collaborator

@leandrocp leandrocp commented Dec 19, 2021

While working on #9 I realized that instead of "display the status-online icon in place of the duration measurement when the liveview is connected" we could display the event duration which is helpful to debug slow events on development. WDYT?

@leandrocp leandrocp requested a review from mcrumm December 21, 2021 22:30
@mcrumm
Copy link
Owner

mcrumm commented Dec 24, 2021

Hey @leandrocp, I like the idea but I want to make sure the values are very clear, so I do not think we should override the total duration. What if we added a third value, something like latest_event_duration? It could override the display in the toolbar.

@leandrocp
Copy link
Collaborator Author

Hey @leandrocp, I like the idea but I want to make sure the values are very clear, so I do not think we should override the total duration. What if we added a third value, something like latest_event_duration? It could override the display in the toolbar.

Actually I agree, I started with a different value but wasn't sure if that was the right approach so I took the simplest idea. Anyway, I'm gonna update the PR 👍🏻

@leandrocp
Copy link
Collaborator Author

Hey @leandrocp, I like the idea but I want to make sure the values are very clear, so I do not think we should override the total duration. What if we added a third value, something like latest_event_duration? It could override the display in the toolbar.

Done 👍🏻

@mcrumm mcrumm merged commit f0fe39c into main Jan 19, 2022
@mcrumm
Copy link
Owner

mcrumm commented Jan 19, 2022

This is awesome

@mcrumm mcrumm deleted the lcp-display-event-duration branch January 19, 2022 17:58
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.

2 participants