Skip to content

dep ensure on cache miss in circleci#1903

Merged
istio-merge-robot merged 2 commits intomasterfrom
urgent_fix
Nov 29, 2017
Merged

dep ensure on cache miss in circleci#1903
istio-merge-robot merged 2 commits intomasterfrom
urgent_fix

Conversation

@rshriram
Copy link
Copy Markdown
Member

Pin Istio/api and istio/fortio to particular SHAs in Gopkg.toml. Otherwise, builds in circleci will continue using old cached version of Gopkg.lock, which will prevent people from updating istio/api and applying the updates here.

@istio-testing
Copy link
Copy Markdown
Collaborator

@rshriram: Adding do-not-merge/release-note-label-needed because the release note process has not been followed.

Details

One of the following labels is required "release-note", "release-note-action-required", or "release-note-none".
Please see: https://github.com/kubernetes/community/blob/master/contributors/devel/pull-requests.md#write-release-notes-if-needed.

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.

@codecov
Copy link
Copy Markdown

codecov bot commented Nov 28, 2017

Codecov Report

Merging #1903 into master will decrease coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1903      +/-   ##
==========================================
- Coverage    81.2%   81.16%   -0.05%     
==========================================
  Files         191      190       -1     
  Lines       19455    18307    -1148     
==========================================
- Hits        15799    14859     -940     
+ Misses       3194     3051     -143     
+ Partials      462      397      -65
Flag Coverage Δ
#broker 45.51% <ø> (ø) ⬆️
#mixer 83.21% <ø> (+0.75%) ⬆️
#pilot 78.19% <ø> (-2.32%) ⬇️
#security 90.39% <ø> (ø) ⬆️
Impacted Files Coverage Δ
pilot/proxy/envoy/fault.go
pilot/proxy/envoy/ingress.go
pilot/proxy/envoy/discovery.go
pilot/proxy/envoy/header.go
pilot/proxy/envoy/mixer.go
pilot/proxy/envoy/config.go
pilot/tools/version/version.go
pilot/proxy/envoy/route.go
pilot/proxy/envoy/watcher.go
pilot/proxy/envoy/policy.go
... and 13 more

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 d5d7d57...ef8ba0b. Read the comment docs.

@rshriram
Copy link
Copy Markdown
Member Author

/test istio-presubmit

@mandarjog
Copy link
Copy Markdown
Contributor

What happens if we remove it from the lock file?
Will circle ci not incur a false cache hit?

I see why it was floated to semver master.

@rshriram
Copy link
Copy Markdown
Member Author

what do you mean? We only care about the versions of specific dependencies in gopkg.toml.. So I make circleci cache key off gopkg.toml only..

@mandarjog
Copy link
Copy Markdown
Contributor

mandarjog commented Nov 29, 2017

@costinm @andraxylia Unpinning istio/api from Gopkg.toml makes it inconsistent with bazel, so making lock file pin makes sense.

Is there a reason why you tolerate this divergence from bazel?

@rshriram
Copy link
Copy Markdown
Member Author

I am not unpinning anything. I have pinned it to a specific version in toml

@kyessenov
Copy link
Copy Markdown
Contributor

Thanks. I've objected to irreproducible master pin in the previous change.
/lgtm

@istio-merge-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kyessenov

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

Details Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@kyessenov
Copy link
Copy Markdown
Contributor

/release-note-none

@istio-merge-robot
Copy link
Copy Markdown

/test all [submit-queue is verifying that this PR is safe to merge]

@istio-merge-robot
Copy link
Copy Markdown

Automatic merge from submit-queue.

@istio-merge-robot istio-merge-robot merged commit ab76cee into master Nov 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants