Layout omit image.ref.name annotation #870
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
also ccing @imjasonh @jonjohnsonjr 🙈 |
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #870 +/- ##
==========================================
- Coverage 52.42% 52.36% -0.06%
==========================================
Files 43 43
Lines 3325 3344 +19
==========================================
+ Hits 1743 1751 +8
- Misses 1353 1363 +10
- Partials 229 230 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
/check-cla |
|
kindly ping @imjasonh ☝️ |
pkg/publish/layout_test.go
Outdated
| { | ||
| name: "no tags", | ||
| tags: nil, | ||
| want: nil, |
There was a problem hiding this comment.
want is nil in all of these cases. If none of these cases test the error scenario we can just remove want from the struct here.
pkg/publish/layout.go
Outdated
| } | ||
|
|
||
| // NewLayout returns a new publish.Interface that saves images to an OCI Image Layout. | ||
| func NewLayout(path string) (Interface, error) { | ||
| func NewLayout(path string, tags []string) (Interface, error) { |
There was a problem hiding this comment.
FYI this changes the method signature, and will break anyone that uses this as a library. I don't think this is a blocker, but if we expect that to be something people do more in the future we should be more careful.
Not a blocker this time though.
There was a problem hiding this comment.
We can change it as a variadic function, this does not affect people who might be using the ko project as a library.
There was a problem hiding this comment.
it should be good now
pkg/publish/layout_test.go
Outdated
| if err != nil { | ||
| t.Fatalf("random.Image() = %v", err) | ||
| } | ||
| importpath := "github.com/Google/go-containerregistry/cmd/crane" |
There was a problem hiding this comment.
This doesn't have to be a real importpath, right? Could we make it something more obviously a placeholder, to avoid confusion? Something like github.com/example/importpath?
developer-guy
force-pushed
the
publish-omit-anns
branch
from
0df8f7d to
e072b66
Compare
|
kindly ping @Dentrax, we should fix the tests 🙈 |
developer-guy
force-pushed
the
publish-omit-anns
branch
from
e072b66 to
b3267ff
Compare
|
kindly ping @imjasonh 🙈 |
|
kindly ping @imjasonh 🕺🏻 |
|
@jonjohnsonjr any concerns? |
pkg/publish/layout.go
Outdated
| p, err := layout.FromPath(path) | ||
| if err != nil { | ||
| p, err = layout.Write(path, empty.Index) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| } | ||
| return &LayoutPublisher{p}, nil | ||
| if len(tags) == 0 { | ||
| tags = []string{"latest"} |
There was a problem hiding this comment.
I'd drop this so it's possible to use this without adding tags?
There was a problem hiding this comment.
@Dentrax are you still interested in working on this?
|
This Pull Request is stale because it has been open for 90 days with |
developer-guy
force-pushed
the
publish-omit-anns
branch
from
b3267ff to
2cec78b
Compare
Fixes ko-build#869 Signed-off-by: Furkan <[email protected]> Co-authored-by: Batuhan <[email protected]> Signed-off-by: Batuhan Apaydın <[email protected]>
developer-guy
force-pushed
the
publish-omit-anns
branch
from
2cec78b to
89d46c8
Compare
|
kindly ping @jonjohnsonjr @imjasonh, it seems that everything is working fine. |
|
kindly ping @Dentrax |
|
This Pull Request is stale because it has been open for 90 days with |
Fixes #869
Signed-off-by: Furkan [email protected]
Co-authored-by: Batuhan [email protected]
cc @developer-guy
DEMO
{ "schemaVersion": 2, "manifests": [ { "mediaType": "application/vnd.docker.distribution.manifest.v2+json", "size": 1943, "digest": "sha256:99a7a7bf1ad3fde0107b1f5909bb9a1df037f8be8b73e73d1779a0fd473cf1ed", "annotations": { "org.opencontainers.image.ref.name": "foo" } }, { "mediaType": "application/vnd.docker.distribution.manifest.v2+json", "size": 1943, "digest": "sha256:99a7a7bf1ad3fde0107b1f5909bb9a1df037f8be8b73e73d1779a0fd473cf1ed", "annotations": { "org.opencontainers.image.ref.name": "bar" } }, { "mediaType": "application/vnd.docker.distribution.manifest.v2+json", "size": 1943, "digest": "sha256:99a7a7bf1ad3fde0107b1f5909bb9a1df037f8be8b73e73d1779a0fd473cf1ed", "annotations": { "org.opencontainers.image.ref.name": "baz" } } ] }