-
Notifications
You must be signed in to change notification settings - Fork 127
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
feat: add v2alpha3 format #1031
Conversation
Skipping CI for Draft Pull Request. |
cf08970
to
69ce97f
Compare
e83c7cb
to
9472ec3
Compare
Thanks @wlynch for the reviews on #1016! 🎊 This PR is the second part of the in the v1beta1 -> v1 tektoncd/chains work split that was identified here: Once this is in, the workstream there should be complete |
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.
❤️
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chitrangpatel, vdemeester 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 |
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.
Overall looks good!
pkg/chains/formats/slsa/v2alpha3/internal/external_parameters/external_parameters.go
Outdated
Show resolved
Hide resolved
The following is the coverage report on the affected files.
|
ba00fb1
to
940bf10
Compare
I believe all review feedback has been addressed now:
lmk if there is any additional feedback or items to address. Thanks! |
The following is the coverage report on the affected files.
|
940bf10
to
39c20f6
Compare
The following is the coverage report on the affected files.
|
9c79b50
to
f4df462
Compare
The following is the coverage report on the affected files.
|
f4df462
to
8d83d12
Compare
The following is the coverage report on the affected files.
|
8d83d12
to
cf252c0
Compare
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.
I think there's probably more refactoring we can do to leverage TektonObject to reduce duplication across v2alpha2 and v2alpha3, but I won't let this block the PR. LGTM!
/lgtm
Fixes #1026
This PR adds a
v2alpha3
format which uses Tektonv1
object(s) in it's Payload information. For backwards compatibility, v2alpha2 and below versions use Tektonv1beta1
object(s) in their Payload information so this is first/only format currently that uses Tektonv1
objects E2E (doesn't convert to v1beta1 informat/*
)This PR is the second part of the in the v1beta1 -> v1 tektoncd/chains work split that was identified here:
#1016 (review)
Once this is in, the workstream there should be complete