-
Notifications
You must be signed in to change notification settings - Fork 55
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: Git optional in CI job pipeline #1505
base: main
Are you sure you want to change the base?
Conversation
Kudos, SonarCloud Quality Gate passed!
|
Coverage report❌ An unexpected error occurred. For more details, check console
Test suite run failedFailed tests: 17/141. Failed suites: 9/41.
Report generated by 🧪jest coverage report action from 810aefa |
@@ -20,6 +21,7 @@ export interface BuildDetails { | |||
tagsEditable: boolean | |||
hideImageTaggingHardDelete: boolean | |||
fetchIdData: FetchIdDataStatus | |||
pipeline:CIPipeline |
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.
Please format this line
@@ -443,7 +440,8 @@ function ciPipelineToNode(ciPipeline: CiPipeline, dimensions: WorkflowDimensions | |||
isRegex: ciMaterial?.isRegex, | |||
primaryBranchAfterRegex: ciMaterial?.source?.value, | |||
cipipelineId: ciMaterial?.id, | |||
isJobCI: ciPipeline?.pipelineType === CIPipelineBuildType.CI_JOB | |||
isJobCI: ciPipeline?.pipelineType === CIPipelineBuildType.CI_JOB, |
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.
We can remove the optional check
if (ci) { | ||
return _nodes.map((node: NodeAttr) => { | ||
if (node.type == WorkflowNodeType.GIT) { | ||
return this.renderSourceNode(node, ci) | ||
return ((node.isJobCI && node.isGitRequired) || !node.isJobCI) && this.renderSourceNode(node, ci) |
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.
We can write it as return (!node.isJobCI || node.isGitRequired) &&
|
||
renderNodes() { | ||
const ci = this.props.nodes.find((node) => node.type == WorkflowNodeType.CI) | ||
const webhook = this.props.nodes.find((node) => node.type == WorkflowNodeType.WEBHOOK) | ||
const _nodesData = this.getNodesData(ci?.id || webhook?.id || '') | ||
const _nodes = _nodesData.nodes | ||
|
||
const _nodes =this.getPositionedNodes( JSON.parse(JSON.stringify(_nodesData.nodes))) |
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.
Format this line. Why are we doing the stringify and parse here?
handleOnBlur={handleOnBlur} | ||
/> | ||
{isJobCI && renderGitRepoToggle()} | ||
{((isJobCI && isGitRequired) || !isJobCI) && ( |
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.
we can write it as {(!isJobCI || isGitRequired) && (
<div className="w-32 h-20"> | ||
<Toggle | ||
selected={isGitRequired} | ||
onSelect={() => { |
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.
Please extract this method out.
@@ -577,6 +584,8 @@ export default function CIPipeline({ | |||
false, | |||
formData.webhookConditionList, | |||
formData.ciPipelineSourceTypeOptions, | |||
|
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.
Please remove this unnecessary space
Description
This makes cloning and displaying git material based on user choice
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist: