Skip to content
This repository has been archived by the owner on Feb 6, 2024. It is now read-only.

Inline utils from rules_docker go pkg #725

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mrmeku
Copy link

@mrmeku mrmeku commented Apr 7, 2023

For #723

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mrmeku
Once this PR has been reviewed and has the lgtm label, please assign michelle192837 for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link

@alexeagle alexeagle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome nice to see you again @mrmeku :)

@@ -12,7 +12,6 @@ import (
"strings"

"github.com/bazelbuild/rules_docker/container/go/pkg/compat"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about this one, should it be inlined too? I guess you'll have a later PR to drop the go_repository that fetches rules_docker code. I don't know what is the maintenance story for rules_k8s so maybe we should wait-and-see if we get any responses.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, maybe that one should also be inlined as part of this change. I had held off since that particular import needs to be translated to understand the OCI image format rather than the format rules_docker is outputting today. But that translation could easily be done in a followup PR after inlining in this one

@alexeagle
Copy link

@chases2 hi, are you the maintainer here?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants