Add change to the artifact packaged event #73
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The artifact packaged event is typically produced by the build process, and should provide details about the source the artifact was built from. While working on the artifact change field, I realised that for the change and branch events the id + source combination should also contain the repository details, otherwise it wouldn't uniquely identify a change. I added a note in the change/branch event section and updated the examples accordingly. I think there is still value in have a repository field in those events, but I marked it as optional (it was mandatory before). Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
afrittoli
force-pushed
the
artifact_change
branch
from
e80f63b to
6852824
Compare
erkist
approved these changes
Oct 9, 2022
erkist
left a comment
erkist
left a comment
There was a problem hiding this comment.
I have nit-picky comments only, feel free to ignore
| |-------|------|-------------|----------| | ||
| | id | `String` | Uniquely identifies the subject within the source. | `abcde-12344-a2b35`, `sh0b31b1c02ff458ad9b7b81cbdf8f028bd54699fa151f221d1e8de6817db93427a1234`, `myimage@sha256:123456abcded`, `six-1.14.0-py2.py3-none-any.whl`| | ||
| | source | `URI-Reference` | [source](../spec.md#source) from the context | `staging/tekton`, `tekton-dev-123`| | ||
| | change | `object` | The top change of the repository from which the artifact has been built | `{"id": "527d4a1aca5e8d0df24813df5ad65d049fc8d312", "source": "my-git.example/an-org/a-repo"}`, `{"id": "feature1234", "source": "my-git.example/an-org/a-repo"}` | |
There was a problem hiding this comment.
The term "top change" is a bit ambiguous to me. Maybe something like "The change (tag, commit, revision etc.) of the repository which was used to build the artifact"?
Member
Author
There was a problem hiding this comment.
Uhm, this makes we wonder if we in future we should use "revision" as field name.
I think for 0.1 it's fine to use change as it matches the name of the change events, but perhaps in future we could use a revision field instead
primer.md
Outdated
| This data model is somewhat limiting, but is has allowed us to put more focus | ||
| on the overall structure of the protocol in the first release. | ||
|
|
||
| We plan to extend the data model to more complex scenarios in upcoming releases. |
Member
Author
There was a problem hiding this comment.
Good catch, thank you. Fixed now.
primer.md
Outdated
| each artifact is build from a single repository and each service is deployed | ||
| from a single artifact. | ||
|
|
||
| This data model is somewhat limiting, but is has allowed us to put more focus |
There was a problem hiding this comment.
This -> The current
limiting -> limited? (limiting is fine too)
e-backmark-ericsson
requested changes
Oct 10, 2022
Contributor
e-backmark-ericsson
left a comment
e-backmark-ericsson
left a comment
There was a problem hiding this comment.
I'd like to see some minor adjustments before approving this PR
e-backmark-ericsson
approved these changes
Oct 10, 2022
Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
afrittoli
force-pushed
the
artifact_change
branch
from
4a70a9a to
667ab25
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
The artifact packaged event is typically produced by the build process, and should provide details about the source the artifact was built from.
While working on the artifact change field, I realised that for the change and branch events the id + source combination should also contain the repository details, otherwise it wouldn't uniquely identify a change.
I added a note in the change/branch event section and updated the examples accordingly. I think there is still value in have a repository field in those events, but I marked it as optional (it was mandatory before).
Signed-off-by: Andrea Frittoli andrea.frittoli@gmail.com
Submitter Checklist
As the author of this PR, please check off the items in this checklist: