-
Notifications
You must be signed in to change notification settings - Fork 36
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
Added Multiple pipeline templates in code completion #611
Conversation
Codecov Report
@@ Coverage Diff @@
## master #611 +/- ##
==========================================
- Coverage 68.20% 68.17% -0.04%
==========================================
Files 103 103
Lines 6316 6316
Branches 1146 1146
==========================================
- Hits 4308 4306 -2
- Misses 2008 2010 +2
Continue to review full report at Codecov.
|
@sudhirverma You modify existing template, but #355 require providing additional templates:
The purpose of the issue was having more template, not one complex. |
build/build-snippets.ts
Outdated
@@ -28,10 +28,35 @@ const snippets: { [key: string]: Snippet } = { | |||
body: load('taskrun.yaml'), | |||
}, | |||
'Pipeline': { | |||
prefix: 'Pipeline', | |||
prefix: 'Pipeline-test', |
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.
Why you rename this?
@@ -0,0 +1,9 @@ | |||
apiVersion: tekton.dev/v1beta1 |
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.
It would be better if you combine pipeline-with-buildah-task.yaml, pipeline-with-kubectl-task.yaml and pipeline-with-maven-task.yaml in to one template
@@ -0,0 +1,15 @@ | |||
apiVersion: tekton.dev/v1beta1 |
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.
This pipeline template should be extended, you just referring to commands but template doesn't provide configuration for them.
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
Fix: #355