SED-4157 display execution status over time in execution list#1329
SED-4157 display execution status over time in execution list#1329lucianbaciu wants to merge 31 commits intomasterfrom
Conversation
…ecution-status-over-time-in-execution-list # Conflicts: # projects/step-frontend/src/lib/modules/execution/components/aggregated-tree-node-history/execution-piechart/aggregated-tree-node-statuses-piechart.component.ts
…ecution-status-over-time-in-execution-list # Conflicts: # projects/step-frontend/src/lib/modules/execution/execution.module.ts
…ecution-status-over-time-in-execution-list
Revert "Reapply "disabled linting"" This reverts commit 54a77cb.
Summary of ChangesHello @lucianbaciu, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the user experience by providing a visual timeline of execution statuses directly within the execution list. It introduces a new data structure to track historical execution results and leverages new, modular UI components to render this information effectively. The changes aim to give users a clearer and more immediate understanding of how executions have progressed over time, alongside general code quality improvements. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new feature to display execution history in the execution list, which is a great addition for traceability. The implementation involves significant refactoring, including the creation of several new, well-encapsulated standalone components. The code is generally of high quality, but I've identified a few areas for improvement, mainly concerning potential runtime errors due to unsafe type assumptions, side effects from array mutations, and some leftover code from development that should be cleaned up. Addressing these points will enhance the robustness and maintainability of the new feature.
.../execution/components/aggregated-tree-node-history/aggregated-tree-node-history.component.ts
Outdated
Show resolved
Hide resolved
.../lib/modules/execution/components/execution-history-cell/execution-history-cell.component.ts
Outdated
Show resolved
Hide resolved
.../execution/components/aggregated-tree-node-history/aggregated-tree-node-history.component.ts
Outdated
Show resolved
Hide resolved
...ted-tree-node-history/execution-piechart/aggregated-tree-node-statuses-piechart.component.ts
Outdated
Show resolved
Hide resolved
...s/execution/components/aggregated-tree-node-history/history-nodes/history-nodes.component.ts
Outdated
Show resolved
Hide resolved
...ib/modules/execution/components/execution-history-cell/execution-history-cell.component.html
Outdated
Show resolved
Hide resolved
.../lib/modules/execution/components/execution-history-cell/execution-history-cell.component.ts
Outdated
Show resolved
Hide resolved
...tep-frontend/src/lib/modules/execution/components/execution-list/execution-list.component.ts
Outdated
Show resolved
Hide resolved
|
/gemini review |
…e-in-execution-list
There was a problem hiding this comment.
Code Review
This pull request introduces a feature to display execution status history in the execution list. This is a great addition that improves visibility into execution trends over time. The implementation is well-structured, introducing new reusable components like HistoryNodesComponent and HistoryCellComponent, and refactoring existing ones to be more robust and modern (e.g., converting to standalone components).
My review includes a few suggestions to address a potential regression in the heatmap tooltip, improve type safety by removing unsafe non-null assertions, and some minor code cleanup to remove duplicate imports and redundant code.
...cution/components/schedule-overview/cross-execution-dashboard/heatmap/heatmap.component.html
Outdated
Show resolved
Hide resolved
.../execution/components/aggregated-tree-node-history/aggregated-tree-node-history.component.ts
Outdated
Show resolved
Hide resolved
...execution/components/aggregated-tree-node-history/history-nodes/history-nodes.component.html
Outdated
Show resolved
Hide resolved
...s/execution/components/aggregated-tree-node-history/history-nodes/history-nodes.component.ts
Outdated
Show resolved
Hide resolved
...s/execution/components/aggregated-tree-node-history/history-nodes/history-nodes.component.ts
Outdated
Show resolved
Hide resolved
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a great new feature to display execution status history directly in the execution list. The implementation is well-structured, leveraging new reusable components for displaying history nodes and pie charts, and includes a significant refactoring to support this functionality. I've identified a few areas for improvement, mainly concerning null-safety to prevent potential runtime errors, an incorrect track-by function in a loop, and a minor code duplication in the module definition. Once these points are addressed, this will be a solid addition.
.../execution/components/aggregated-tree-node-history/aggregated-tree-node-history.component.ts
Outdated
Show resolved
Hide resolved
...cution/components/schedule-overview/cross-execution-dashboard/heatmap/heatmap.component.html
Outdated
Show resolved
Hide resolved
| ErrorRootCausesComponent, | ||
| AltExecutionTimePopoverTitleDirective, | ||
| AggregatedTreeNodeStatusesPiechartComponent, | ||
| AggregatedTreeNodeStatusesPiechartComponent, |
…ecution-status-over-time-in-execution-list # Conflicts: # projects/step-frontend/src/lib/modules/execution/execution.module.ts
No description provided.