-
Notifications
You must be signed in to change notification settings - Fork 12
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
Add "example" generated commands (so changes to generated commands are easier to see) #18
Conversation
@@ -12,5 +12,27 @@ set -- docker:cli docker:dind docker:windowsservercore notary # a little bit of | |||
# (see "library/" and ".external-pins/" for where these come from / are hard-coded for consistent testing purposes) | |||
# NOTE: we are explicitly *not* pinning "golang:1.19-alpine3.16" so that this also tests unpinned parent behavior (that image is deprecated so should stay unchanging) | |||
|
|||
time bashbrew fetch "$@" |
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.
Splitting this out from being implied by sources.sh
would've made PRs like #16 more interesting (because we'd see the actual change in generation time, not affected by Git fetch time), but we're doing such a small set here that it's really not much of a difference. 😄
b639da8
to
7aee8cd
Compare
[ | ||
first(.[] | select(normalized_builder == "buildkit")), | ||
first(.[] | select(normalized_builder == "classic")), | ||
first(.[] | select(normalized_builder == "oci-import")), |
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.
It seems like this would select three builds, but there are only two sets of commands generated. Is this because there are no oci-import examples yet?
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.
Yes, that's correct -- I had originally left a # TODO oci-import
here, but realized I could just add the conditional and it would match as soon as there was an example in the test data (which I plan to add when I have an implementation in meta.jq
).
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.
(In other words, this is currently a very fancy and computational way to spell empty
😂)
See #17 (comment)