-
Notifications
You must be signed in to change notification settings - Fork 268
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 matrix TaskRuns on the PipelineRun details page #3081
Conversation
Timed out on CreatePipelineRun test, stuck on loading state for >3 seconds. I've already bumped the timeout for this step in #3040 Re-running for now and will need to find another approach as we can't just keep increasing the timeout. When running locally this step takes ~50ms, so it really shouldn't be taking 3+ seconds (60 times as long…) in CI. /test tekton-dashboard-unit-tests |
Hold for a bit so we can publish a new version of the packages with #3082. I'll unhold once that's done in the next hour or so. |
Ready for review |
selectedTaskId === pipelineTaskName || | ||
(!erroredTask && !selectedTaskId && index === 0); | ||
const selectDefaultStep = !selectedTaskId; | ||
!hasExpandedTask && |
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.
Found this value tough to deconstruct. Perhaps worth adding a comment or two, or splitting up into more variables with explanatory names?
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.
Yeah there's a lot going on there, added comments for each branch. Let me know if that helps
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.
Looks good, thanks
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.
Looks good overall, just one suggestion
Add support for displaying all TaskRuns produced by a matrix task on the PipelineRun details page. Future updates will improve the display of the matrix information making it easier to differentiate between them on the left task/step nav, and making it clearer which params are provided from the matrix.
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.
/lgtm
selectedTaskId === pipelineTaskName || | ||
(!erroredTask && !selectedTaskId && index === 0); | ||
const selectDefaultStep = !selectedTaskId; | ||
!hasExpandedTask && |
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.
Looks good, thanks
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: briangleeson The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Changes
Related to #2410
Add support for displaying all TaskRuns produced by a matrix task on the PipelineRun details page.
Future updates will improve the display of the matrix information making it easier to differentiate between them on the left task/step nav, and making it clearer which params are provided from the matrix.
/kind feature
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
functionality, content, code)
/kind <type>
. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes