Skip to content

Reformat vendor.conf: use columns, pin by git-sha, and sort alphabetically#3858

Merged
crosbymichael merged 3 commits intocontainerd:masterfrom
thaJeztah:reformat_vendor
Dec 2, 2019
Merged

Reformat vendor.conf: use columns, pin by git-sha, and sort alphabetically#3858
crosbymichael merged 3 commits intocontainerd:masterfrom
thaJeztah:reformat_vendor

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah commented Nov 28, 2019

Since it doesn't look like this repo will switch to using go mod soon, let's reformat this file to make comparing with other repositories slightly easier and to encourage pinning

Best to review individual commits (no dependencies were updated, so vendored files should stay unmodified)

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Nov 28, 2019

Build succeeded.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Nov 28, 2019

Build succeeded.

@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Nov 28, 2019

Wait, why is this failing?

script/setup/install-cni: line 31: ./build.sh: No such file or directory
The command "if [ "$TRAVIS_GOOS" = "linux" ]; then sudo PATH=$PATH GOPATH=$GOPATH script/setup/install-cni ; fi" failed and exited with 127 during .

@estesp
Copy link
Copy Markdown
Member

estesp commented Nov 28, 2019

The comment added to the line vendoring CNI makes this grep .. | cut .. command fail is my guess: https://github.com/containerd/containerd/blob/master/script/setup/install-cni#L24

@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Nov 29, 2019

The comment added to the line vendoring CNI makes this grep .. | cut .. command fail is my guess: https://github.com/containerd/containerd/blob/master/script/setup/install-cni#L24

Ah, yes, that looked plausible; updated scripts that were assuming a single space

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Nov 29, 2019

Build succeeded.

@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Nov 29, 2019

ok, looks to be green now, except for a (I think) flaky test;

Ginkgo ran 1 suite in 59.158347938s
3951Test Suite Failed
3952--- FAIL: TestCRISuite (59.23s)
3953    cri_test.go:130: Failed to run tests in parallel: exit status 1
3954FAIL

estesp
estesp approved these changes Nov 29, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov-io
Copy link
Copy Markdown

codecov-io commented Nov 29, 2019

Codecov Report

Merging #3858 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3858   +/-   ##
=======================================
  Coverage   42.34%   42.34%           
=======================================
  Files         130      130           
  Lines       14683    14683           
=======================================
  Hits         6217     6217           
  Misses       7540     7540           
  Partials      926      926
Flag Coverage Δ
#linux 45.76% <ø> (ø) ⬆️
#windows 37.82% <ø> (ø) ⬆️

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 3a31ce2...6fa9a42. Read the comment docs.

@crosbymichael
Copy link
Copy Markdown
Member

crosbymichael commented Dec 2, 2019

LGTM

@crosbymichael crosbymichael merged commit f2ef7b7 into containerd:master Dec 2, 2019
@thaJeztah thaJeztah deleted the reformat_vendor branch December 2, 2019 14:26
@thaJeztah thaJeztah mentioned this pull request Mar 5, 2020
Merged
@thaJeztah thaJeztah mentioned this pull request Apr 3, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@estesp estesp estesp approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@thaJeztah @estesp @codecov-io @crosbymichael