-
Notifications
You must be signed in to change notification settings - Fork 248
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
Change clusterTask timeout from int to string and remove -t shorthand #793
Conversation
Hi @kadern0. Thanks for your PR. I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/hold Thanks for this pr @kadern0. Unfortunately, this can't be added until version If you are interested in opening a separate pr, we need to add a warning message about this change to users in |
@danielhelfand what do you suggest? Can this wait until version |
I will assign you to the issue if that works for you so that it's clear to others it's being worked on. We can keep this open/give any feedback until |
Sounds good to me. Thanks! |
/assign @kadern0 |
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.
Adding some feedback here for whenever you have a moment. A good reference for if there's any confusion is to look at how this approach was implemented in #779.
/ok-to-test |
This looks great! Thank you. Will keep you posted on the status of this, but I would say to expect that this will be introduced some time early next month. |
/hold cancel Thanks for your patience @kadern0. Could you please sign the new CLA the Tekton project has adopted in the last month? Apologies, but I think this should be good to go. |
/retest |
Thanks @danielhelfand, I'm happy to sign the new CLA. Could you trigger the bot to verificate so I'm asked for it and I can sign it? (unless there is an easier way to do it) |
/test pull-tekton-cli-integration-tests-0_10 |
Could you try doing a force push? That is unfortunately the only workaround that I have seen. |
|
0b052f1
to
3b34501
Compare
Fixes issue tektoncd#784 Signed-off-by: Pablo Caderno <kaderno@gmail.com>
@danielhelfand, the force push did the trick (although I had another email set by default and it took me a few pushes to find out). Now that this is solved, in order to make my code pass the tests, the following warning has to be removed because
|
The following is the coverage report on the affected files.
|
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
Thanks @kadern0!
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danielhelfand 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 |
#945 | [Piyush Garg] Fix release script and yaml | 2020/04/29-13:39 #946 | [Piyush Garg] Update readme for release 0.9.0 | 2020/04/29-14:24 #948 | [Piyush Garg] Fix dockerfile of debbuild | 2020/04/29-17:43 #793 | [Pablo Caderno] Change clusterTask timeout from int to string and remove -t shorthand | 2020/04/30-14:34 #793 | [Pablo Caderno] Removed unnecessary warning message | 2020/04/30-14:34 #952 | [Daniel Helfand] remove default 1h for clustertask --timeout and add tests for invalid timeout | 2020/04/30-16:55 #954 | [Daniel Helfand] remove tkn create -f | 2020/05/01-07:49 #953 | [Vincent Demeester] Bump plumbing to recent master 👼 | 2020/05/01-08:14 #935 | [praveen4g0] Add Task start E2E tests | 2020/05/01-14:06 null | [Anshul Verma] Provides an interactive Prompt to select a Pipeline/PipelineRun/PipelineResources/Task/TaskRun/ClusterTask/ to describe just like what is there in subcommand. | 2020/05/04-07:41 null | [Pradeep Kumar] fix task start with file | 2020/05/06-14:07 null | [Yulia Gaponenko] Update Readme with s390x build usage | 2020/05/07-09:53 null | [Vibhav Bobade] Update triggerbinding and template `list` error messages | 2020/05/08-15:56 null | [Piyush Garg] Enable yaml lint | 2020/05/10-10:19 null | [Piyush Garg] Refactoring for --all-namespace flag | 2020/05/10-10:33 null | [Piyush Garg] Makefile changes for new skipCheckFlag | 2020/05/10-11:38 null | [vinamra28] Fix to provide an error for tkn completion without argument | 2020/05/13-14:00 null | [charles-edouard.breteche] refacto get pipelines version | 2020/05/14-14:01 null | [savitaashture] Add triggers version info to the version command | 2020/05/15-17:25 null | [Daniel Helfand] switch to sigs.k8s.io/yaml for yaml unmarshal | 2020/05/19-04:38 null | [Alan Greene] Fix 'Edit this page' and 'Create an issue' links on the website | 2020/05/19-04:45 null | [Daniel Helfand] move e2e tests into subcommand packages | 2020/05/19-07:53 null | [Shivam Mukhade] Add Task's timeout and Params to Pipeline Desc | 2020/05/19-10:30 null | [Vibhav Bobade] Add no-headers and all-namespaces flags for pipeline resources | 2020/05/19-15:20 null | [Anshul Verma] Provides `--last` option of the `describe` of PipelineRun/TaskRun to describe the most recent PipelineRun/TaskRun | 2020/05/22-13:08 null | [Daniel Helfand] change taskrun and pipelinerun delete test names | 2020/05/26-11:54 null | [Divyansh42] Fix tkn task delete <name> --trs deletes ClusterTaskrun also | 2020/05/26-12:06 null | [chetan-rns] Display conditions in the pipeline describe command | 2020/05/26-12:17 null | [Pradeep Kumar] Task start interactive mode | 2020/05/26-13:53 null | [vinamra28] Fix to provide error in case of no pr logs | 2020/05/26-14:20 null | [Piyush Garg] Bump tektoncd/pipeline to v0.12.1 | 2020/05/28-06:09 null | [Piyush Garg] Bump tektoncd/triggers to v0.5.0 | 2020/06/02-09:28 null | [Divyansh42] Add `--trs` flag in `tkn ct delete` command | 2020/06/03-14:34 null | [savitaashture] Modified TriggerTemplate describe command for ResourceTemplates | 2020/06/05-09:29 null | [Daniel Helfand] fix --keep with tr and pr delete when using --task or --pipeline flag | 2020/06/10-09:05 null | [Chmouel Boudjnah] Fix Debian package having an incorrect package name | 2020/06/10-10:06 null | [Chmouel Boudjnah] Decrease minor release version to zero | 2020/06/10-10:06 null | [Piyush Garg] Bump tektoncd/pipeline to v0.13.0 | 2020/06/10-14:18 Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
Signed-off-by: kadern0 kaderno@gmail.com
Changes
Changed clusterTask timeout from int64 to string and removed the shorthand
-t
option.Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
make check
make generated
See the contribution guide
for more details.
Release Notes