Update API spec doc to reflect the new fields introduced from TEP75&76
Changes
Implementation of TEP75&76 supported object type param, object type result and array type result, which introduced some changes to the API spec i.e. ParamSpec, TaskResult and TaskRunResult.
This PR updates the doc to reflect the changes.
Signed-off-by: Chuang Wang [email protected]
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
- [ ] Has Docs included if any changes are user facing
- [ ] Has Tests included if any functionality added or changed
- [ ] Follows the commit message standard
- [ ] Meets the Tekton contributor standards (including functionality, content, code)
- [ ] Has a kind label. You can add one by adding a comment on this PR that contains
/kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep - [ ] Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
- [ ] Release notes contains the string "action required" if the change requires additional action from users switching to the new release
Release Notes
NONE
@Yongxuanzhang Please take a look. Let me know if I miss anything. Thanks
/kind doc
@chuangw6: The label(s) kind/doc cannot be applied, because the repository doesn't have them.
In response to this:
/kind doc
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.
/kind documentation
/approve
I don't think we want to mark array results and object params + results as required for conformance especially since these fields are still in alpha. TEP-0012 says that we may add "an optional process to very mature supported features, to potentially mark them as required by the spec", and TEP-0033 states that "Fields should not be added to the Tekton conformance surface until they are promoted to the same stability level as the CRD impacted". This doc also states that "In general a resource or field should not be added as REQUIRED directly, as this may cause unsuspecting previously-conformant implementations to suddenly no longer be conformant". Maybe "recommended" would be better.
I don't think we want to mark array results and object params + results as required for conformance especially since these fields are still in alpha. TEP-0012 says that we may add "an optional process to very mature supported features, to potentially mark them as required by the spec", and TEP-0033 states that "Fields should not be added to the Tekton conformance surface until they are promoted to the same stability level as the CRD impacted". Maybe "recommended" would be better.
Thanks for the pointers @lbernick ! We are not marking specifically array result, object param + result as required. It's the type field. And I agree that this is indeed not a required field since the mutating webhook will set it if not provided. In addition, I feel like some other fields marked as required should also be changed to recommended or optional i.e. description, paramSpec's default. wdyt?
Thanks for the pointers @lbernick ! We are not marking specifically array result, object param + result as required. It's the
typefield.
If supporting "type" is required, doesn't that imply that supporting object params + results is required? It wouldn't make sense to allow users to define a "type" but have it not do anything. Unless the only "type" value that is required to be supported is type string? This is making me realize that for enum fields, we probably need to specify which values of the enum should be supported, rather than specifying conformance requirements for the field as a whole.
And I agree that this is indeed not a required field since the mutating webhook will set it if not provided.
This is a detail of our current implementation of the Tekton API, but the conformance spec dictates what fields all implementations must support.
In addition, I feel like some other fields marked as
requiredshould also be changed torecommendedoroptionali.e.description, paramSpec'sdefault. wdyt?
I think "description" and "default" have been around for much longer and we are much more confident in the stability of those fields. Since we already have these fields as required, I don't see a good reason to downgrade them to "recommended".
@lbernick I see. I've updated to mark the type field in TaskResult and TaskRunResult as recommended for the object and array.
PTAL! Thanks
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: abayer, lbernick
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [abayer,lbernick]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
/lgtm since andrew previously approved.
/lgtm
/test check-pr-has-kind-label
@chuangw6: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:
/test pull-tekton-pipeline-alpha-integration-tests/test pull-tekton-pipeline-build-tests/test pull-tekton-pipeline-integration-tests/test tekton-pipeline-unit-tests
The following commands are available to trigger optional jobs:
/test pull-tekton-pipeline-go-coverage
Use /test all to run all jobs.
In response to this:
/test check-pr-has-kind-label
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.
/retest