Skip to content

Conversation

liggitt
Copy link
Member

@liggitt liggitt commented May 13, 2019

This reverts commit e25e5a6.

What type of PR is this?
/kind bug

What this PR does / why we need it:

Which issue(s) this PR fixes:
#77055 made changes that rendered the verify-no-vendor-cycles.sh check ineffective (it passed immediately on #77837 which introduced vendor cycles)

Does this PR introduce a user-facing change?:

NONE

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 13, 2019
@liggitt
Copy link
Member Author

liggitt commented May 13, 2019

/priority critical-urgent

@k8s-ci-robot k8s-ci-robot added priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. approved Indicates a PR has been approved by an approver from all required OWNERS files. release-note-none Denotes a PR that doesn't merit a release note. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels May 13, 2019
@k8s-ci-robot k8s-ci-robot requested review from eparis and sttts May 13, 2019 23:15
@liggitt
Copy link
Member Author

liggitt commented May 13, 2019

/cc @fejta @davidz627

Copy link
Member

@BenTheElder BenTheElder left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. kind/bug Categorizes issue or PR as related to a bug. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels May 13, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BenTheElder, liggitt

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

@BenTheElder
Copy link
Member

/kind bug
/sig testing

@k8s-ci-robot k8s-ci-robot added sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels May 13, 2019
@davidz627
Copy link
Contributor

lol amazing. Thanks for the fix :)

Copy link
Member

@BenTheElder BenTheElder left a comment

Choose a reason for hiding this comment

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

reverse reviewed for any future attempt at patching this

staging_repos=()
while IFS= read -r repo; do
staging_repos+=( "${repo}" )
done < <(ls "${KUBE_ROOT}/staging/src/k8s.io/")
Copy link
Member

Choose a reason for hiding this comment

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

for the future: this should be using find instead of ls

echo "Package ${i} has a cyclic dependency on staging repository packages: ${deps_on_staging}"
failed=true
fi
done < <(find vendor/ -type d)
Copy link
Member

@BenTheElder BenTheElder May 13, 2019

Choose a reason for hiding this comment

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

this should be using -print0

Copy link
Member

Choose a reason for hiding this comment

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


failed=false
while IFS= read -r -d '' i; do
deps=$(go list -f '{{range .Deps}}{{.}}{{"\n"}}{{end}}' ./"$i" 2> /dev/null || echo "")
Copy link
Member

Choose a reason for hiding this comment

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

should be quoting the whole string

set -o nounset
set -o pipefail

KUBE_ROOT=$(dirname "${BASH_SOURCE[0]}")/..
Copy link
Member

Choose a reason for hiding this comment

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

this change was right

@k8s-ci-robot k8s-ci-robot merged commit 5f72845 into kubernetes:master May 14, 2019
@liggitt liggitt deleted the restore-verify-script branch May 14, 2019 03:02
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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. release-note-none Denotes a PR that doesn't merit a release note. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants