Skip to content
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

Removing workaround to disable selinux label - updated #645

Merged

Conversation

Shashankft9
Copy link
Member

@Shashankft9 Shashankft9 commented Nov 9, 2021

Changes

/kind cleanup

Fixes #390 (comment)

I came across this code in builder.go few days ago, and saw in the comments that it was a workaround till the issue mentioned above is fixed, so did a fix here: buildpacks/pack#1307 and the next logical thing seemed to cleanup the code, so I tried that here - please let me know if this is not required.

@knative-prow-robot
Copy link

@Shashankft9: The label(s) kind/cleanup cannot be applied, because the repository doesn't have them.

In response to this:

Changes

/kind cleanup

Fixes #390 (comment)

I came across this code in builder.go few days ago, and saw in the comments that it was a workaround till the issue mentioned above is fixed, so did a fix here: buildpacks/pack#1307 and the next logical thing seemed to cleanup the code, so I tried that here - please let me know if this is not required.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@google-cla google-cla bot added the cla: yes Indicates the PR's author has signed the CLA. label Nov 9, 2021
@knative-prow-robot knative-prow-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Nov 9, 2021
@codecov
Copy link

codecov bot commented Nov 9, 2021

Codecov Report

Merging #645 (cbc3578) into main (c810efc) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #645   +/-   ##
=======================================
  Coverage   36.26%   36.26%           
=======================================
  Files          38       38           
  Lines        3847     3847           
=======================================
  Hits         1395     1395           
  Misses       2268     2268           
  Partials      184      184           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c810efc...cbc3578. Read the comment docs.

// Client with a logger which is enabled if in Verbose mode.
packClient, err := pack.NewClient(pack.WithLogger(logging.New(logWriter)), pack.WithDockerClient(dockerClientWrapper))
packClient, err := pack.NewClient(pack.WithLogger(logging.NewSimpleLogger(logWriter)))
Copy link
Contributor

Choose a reason for hiding this comment

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

@Shashankft9 custom docker client cli still should be set in order to support SSH.

@matejvasek
Copy link
Contributor

Apart for the single comment this looks good.

@matejvasek
Copy link
Contributor

/approve

@knative-prow-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: matejvasek, Shashankft9

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 9, 2021
@matejvasek
Copy link
Contributor

/hold until CI pass

@knative-prow-robot knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 9, 2021
@matejvasek
Copy link
Contributor

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 9, 2021
@matejvasek
Copy link
Contributor

/unhold

@knative-prow-robot knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 9, 2021
@knative-prow-robot knative-prow-robot merged commit 1bbef7a into knative:main Nov 9, 2021
jrangelramos pushed a commit to jrangelramos/func that referenced this pull request Aug 24, 2023
* deps: update tekton to latest versions (knative#1753)

Fixes: knative#1716

Signed-off-by: Lance Ball <lball@redhat.com>
Signed-off-by: Matej Vasek <mvasek@redhat.com>

* fixup: update CI Go version

Signed-off-by: Matej Vasek <mvasek@redhat.com>

* fixup: update golangci-lit version

Signed-off-by: Matej Vasek <mvasek@redhat.com>

* fixup: update github.com/tektoncd/cli

Signed-off-by: Matej Vasek <mvasek@redhat.com>

* fixup: delete replacement of tekton lib version

Signed-off-by: Matej Vasek <mvasek@redhat.com>

---------

Signed-off-by: Lance Ball <lball@redhat.com>
Signed-off-by: Matej Vasek <mvasek@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants