-
Notifications
You must be signed in to change notification settings - Fork 902
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
feat(core/ManualJudgment): Enhanced ManualJudgment #8818
feat(core/ManualJudgment): Enhanced ManualJudgment #8818
Conversation
bb396c9
to
652a11c
Compare
2d6134b
to
cab7df5
Compare
40624c5
to
1287ebf
Compare
@caseyhebebrand @vigneshm @christopherthielen Could you please review the code. |
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.
I left some inline comments, but a couple more general observations that would be helpful:
- Defining a more explicit
type
for manualJudgment. Usingany
is too vague, especially for an entity that has so many helper operations - Some of the helper functions are pretty complex (
crossAppDataFetch
,nestedManualJudgment
). I would encourage you to either add supplementary comments, break these into smaller functions, or see if you can reduce any repeated logic that exists between components.
app/scripts/modules/core/src/pipeline/executions/execution/Execution.tsx
Outdated
Show resolved
Hide resolved
app/scripts/modules/core/src/pipeline/executions/execution/Execution.tsx
Outdated
Show resolved
Hide resolved
app/scripts/modules/core/src/projects/dashboard/pipeline/ProjectPipeline.tsx
Outdated
Show resolved
Hide resolved
app/scripts/modules/core/src/pipeline/executions/execution/executionMarker.less
Outdated
Show resolved
Hide resolved
app/scripts/modules/core/src/pipeline/executions/execution/ExecutionMarker.tsx
Outdated
Show resolved
Hide resolved
app/scripts/modules/core/src/pipeline/executions/execution/ExecutionMarker.tsx
Outdated
Show resolved
Hide resolved
app/scripts/modules/core/src/pipeline/executions/execution/ExecutionMarker.tsx
Outdated
Show resolved
Hide resolved
app/scripts/modules/core/src/pipeline/executions/execution/ExecutionMarker.tsx
Outdated
Show resolved
Hide resolved
bd0592a
to
5f4fe2a
Compare
…ick view with backend support
Pr/code enhancement
app/scripts/modules/core/src/pipeline/executions/execution/ExecutionMarker.tsx
Outdated
Show resolved
Hide resolved
app/scripts/modules/core/src/pipeline/executions/execution/ExecutionMarker.tsx
Outdated
Show resolved
Hide resolved
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.
Thanks for responding to all the feedback!
Goals and Non-Goals
Add a filter for filtering pipelines waiting on manual judgment. Checking this filter will automatically select the running stage filter and disable it(since all manual judgment pipelines are running). On deselecting the manual judgment filter the running filter will be enabled but still selected.
There will be a visual indication on all the stages that are waiting on manual judgment and clicking on the stage will take you to the root pipeline waiting on manual judgment.
Design
Enhanced ExecutionMarker.tsx to
-> If the leaf node of a pipeline execution waiting for judgment exist then reusing the UiRef component for redirection
to leaf node waiting for manual judgment.
-> If the stage durations setting is disabled, show a waiting icon.
Enhanced executionMarker.less to
-> Added a visual indicator to the stages waiting on manual judgment.