Add outcome to buildfinished event #179
Conversation
This commit adds the outcome enum to the buildfinished event Signed-off-by: benjamin-j-powell <[email protected]>
| "outcome": { | ||
| "type": "string", | ||
| "minLength": 1, | ||
| "enum": [ |
There was a problem hiding this comment.
The biggest difference here between outcome in taskRun.finished is this. I really think we should be utilizing enums to provide at least some consistent defaults for users.
There was a problem hiding this comment.
Absolutely! And that is also stated for the taskRun.finished and pipelineRun.finished in their md doc (core.md). The json schemas for those events lack those enum definitions, and there is an issue to fix that - #159 . I believe the outcome enums for the build.finished event should have similar values as the enum for the outcomes of taskRun.finished and pipelineRun.finished.
| } |
There was a problem hiding this comment.
Unsure why we want this non-newline for files? Newlines at the ending character is pretty standard, e.g. vim, emacs, all defaults to this (and IDEs). So this may have been due to this being code generated and forgetting the newline? Im unsure.
There was a problem hiding this comment.
Yes, all files should end with an empty line I believe
schemas/buildfinished.json
Outdated
| @@ -20,9 +20,9 @@ | |||
| "type": { | |||
| "type": "string", | |||
| "enum": [ | |||
| "dev.cdevents.build.finished.0.1.1" | |||
| "dev.cdevents.build.finished.0.1.2" | |||
Signed-off-by: benjamin-j-powell <[email protected]>
| @@ -3,15 +3,16 @@ | |||
| "version": "0.4.0-draft", | |||
| "id": "271069a8-fc18-44f1-b38f-9d70a1695819", | |||
| "source": "/event/source/123", | |||
| "type": "dev.cdevents.build.finished.0.1.1", | |||
| "type": "dev.cdevents.build.finished.0.1.2", | |||
There was a problem hiding this comment.
-draft is needed here too
| "enum": [ | ||
| "passed", | ||
| "failed", | ||
| "cancelled" |
There was a problem hiding this comment.
We might want to describe the meaning of values in the enum, to help consistency in adoption.
Does cancelled refer to use cancelled or system cancelled or both? Would a "timed-out" build count as cancelled (as in cancelled by the system because of taking too long?).
|
@xibz - |
| "type": "string", | ||
| "minLength": 1, | ||
| "enum": [ | ||
| "passed", |
There was a problem hiding this comment.
These enums don't match the core.md doc on pipelineRun.finished. Same for taskRun.finished.
|
This PR overlaps somewhat with #238 . Should they be combined? |
|
See also #215 |
Changes
This PR addresses a missing field of,
outcome, that should exist in thebuild.finishedevent.This follows the
outcomeoftaskRun.finished, except that it utilizes an enum value instead. Users should be able to pass any string they want to, but we should also have some sane enums so consumers and producers do not have several ways of representing the same thing.Submitter Checklist
As the author of this PR, please check off the items in this checklist: