-
Notifications
You must be signed in to change notification settings - Fork 27
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
Design for generic artifact fetching #652
base: main
Are you sure you want to change the base?
Design for generic artifact fetching #652
Conversation
@brunoapimentel PTAL |
1ec4342
to
ae7a50c
Compare
65628a0
to
b6fd32b
Compare
ee5d0bd
to
2ceeaa7
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.
Leaving some additional comments just to clarify some points, but I believe we have a solid direction to start the implementation.
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.
Nothing major just quick nitpick fixes. In general I am in agreement. With the first ideas of contributing guidelines having materialized recently we can merge this OR keep it open until the implementation work is done, update sections as necessary should some further technical considerations be made during development, etc. I'd be fine either way. That said, if you wish to merge this right away then please squash all commits into a single one, no fixups please.
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.
Giving my ack here since I'm also in agreement with the design, but I'd suggest not to merge this (as Erik commented before), and turn it in an ADR when the feature is production-ready.
+1 |
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 +1, thank you guys!
Signed-off-by: Jan Koscielniak <[email protected]>
Signed-off-by: Jan Koscielniak <[email protected]>
Changelog: - lockfile has two ways of specifying the url - add a note about ExternalReferences in the SBOM - explicitely reference EC rule used to validate download url - formatting Signed-off-by: Jan Koscielniak <[email protected]>
Signed-off-by: Jan Koscielniak <[email protected]>
Signed-off-by: Jan Koscielniak <[email protected]>
Signed-off-by: Jan Koscielniak <[email protected]>
efcd917
to
b1020cf
Compare
Thanks for all your reviews, I'll keep this open and will then edit it into ADR when we're done with the implementation. |
Opened a new PR #718 that converts this content into ADR and user documentation |
Maintainers will complete the following section
Note: if the contribution is external (not from an organization member), the CI
pipeline will not run automatically. After verifying that the CI is safe to run:
/ok-to-test
(as is the standard for Pipelines as Code)