-
Notifications
You must be signed in to change notification settings - Fork 576
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
Adds a tool to create a simple task template #739
base: main
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
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.
/cc @tektoncd/catalog-maintainers
00a675f
to
a00cbac
Compare
/hold cancel |
/test pull-tekton-catalog-integration-tests |
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.
Some nits about code, maybe you would like to do a class and some argpass for passing default argument, you can maybe get inspiration from this script if you want to snatch the parse_args/main and class function
tools/tools.py
Outdated
json_object = json.loads(jsondata) | ||
|
||
# Type of resource | ||
type = input("Enter type of resource: task/pipeline: ").lower() |
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.
type is normally a reserver word in python :D
tools/tools.py
Outdated
metadata["annotations"]["tekton.dev/pipelines.minVersion"] = minPipelineVersion | ||
metadata["annotations"]["tekton.dev/displayName"] = displayName | ||
|
||
except OSError as error: |
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 don't you just os.path.exists the file instead of a try block?
tools/tools.py
Outdated
metadata["annotations"]["tekton.dev/displayName"] = displayName | ||
|
||
except OSError as error: | ||
print("""Resource with %s and version %s already exists""" % |
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.
you can use fstring instead
I think ideally it would be best to include a README template as well and have a few argument, we could as plug inside cookicutter which would get us a lot of lifting : https://github.com/cookiecutter/cookiecutter For example for a default template on django https://github.com/pydanny/cookiecutter-django Another idea, would be to just do this in go inside catlin (with go 1.5 embed stuff), having the validator near the linter (ie: same repo) is not a bad idea i think, so we can update easily the lint and the default template in a commit, |
optional: true # Optional value of the workspace | ||
|
||
params: | ||
- name: '' # Name of parms |
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.
small typo
- name: '' # Name of parms | |
- name: '' # Name of param |
- This patch adds a tool which help to create a manifest template for task/pipeline based on Tekton Catalog Organization Proposal Signed-off-by: Puneet Punamiya <[email protected]>
Issues go stale after 90d of inactivity. /lifecycle stale Send feedback to tektoncd/plumbing. |
/remove-lifecycle stale |
Issues go stale after 90d of inactivity. /lifecycle stale Send feedback to tektoncd/plumbing. |
/remove-lifecycle stale |
since we are moving all these in |
Signed-off-by: Puneet Punamiya [email protected]
Changes
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
File path follows
<kind>/<name>/<version>/name.yaml
Has
README.md
at<kind>/<name>/<version>/README.md
Has mandatory
metadata.labels
-app.kubernetes.io/version
the same as the<version>
of the resourceHas mandatory
metadata.annotations
tekton.dev/pipelines.minVersion
mandatory
spec.description
follows the conventionSee the contribution guide
for more details.