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

define types.BoolValue var in pkg/proto/types.go #9216

Merged
merged 3 commits into from
Nov 8, 2018

Conversation

hzxuzhonghu
Copy link
Member

@codecov
Copy link

codecov bot commented Oct 9, 2018

Codecov Report

Merging #9216 into master will decrease coverage by 1%.
The diff coverage is 84%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #9216    +/-   ##
=======================================
- Coverage      70%     70%   -<1%     
=======================================
  Files         431     431            
  Lines       40488   40352   -136     
=======================================
- Hits        28262   28097   -165     
- Misses      10853   10887    +34     
+ Partials     1373    1368     -5
Impacted Files Coverage Δ
pilot/pkg/networking/plugin/health/health.go 53% <100%> (-1%) ⬇️
pilot/pkg/networking/core/v1alpha3/route/route.go 72% <100%> (ø) ⬆️
mixer/pkg/runtime/routing/builder.go 58% <100%> (ø) ⬆️
mixer/pkg/runtime/config/ephemeral.go 96% <100%> (ø) ⬆️
...ilot/pkg/networking/plugin/authn/authentication.go 79% <100%> (ø) ⬇️
pilot/pkg/networking/core/v1alpha3/httproute.go 86% <100%> (ø) ⬆️
pilot/pkg/networking/core/v1alpha3/gateway.go 47% <72%> (ø) ⬇️
pilot/pkg/networking/core/v1alpha3/listener.go 76% <78%> (+1%) ⬆️
istioctl/pkg/install/verify.go 21% <0%> (-4%) ⬇️
...olarwinds/internal/papertrail/papertrail_logger.go 81% <0%> (-3%) ⬇️
... and 21 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 ba67713...4e59444. Read the comment docs.

@hzxuzhonghu
Copy link
Member Author

/test istio-unit-tests

@@ -24,21 +24,15 @@ import (
"github.com/envoyproxy/go-control-plane/envoy/api/v2/route"
http_conn "github.com/envoyproxy/go-control-plane/envoy/config/filter/network/http_connection_manager/v2"
"github.com/gogo/protobuf/types"
Copy link
Member

Choose a reason for hiding this comment

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

If move types.BoolValue into pkg/proto/types.go, then this imported package is not needed any more.

Please fix https://github.com/istio/istio/pull/9216/files#diff-9eec7bb8f46726e51ab373e7b8e11ca7L428

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Copy link
Member

@supereagle supereagle left a comment

Choose a reason for hiding this comment

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

LGTM

@istio-testing istio-testing added the needs-rebase Indicates a PR needs to be rebased before being merged label Oct 20, 2018
@hzxuzhonghu
Copy link
Member Author

rebased, ptal

@hzxuzhonghu hzxuzhonghu removed the needs-rebase Indicates a PR needs to be rebased before being merged label Oct 22, 2018
@istio-testing istio-testing added the needs-rebase Indicates a PR needs to be rebased before being merged label Oct 26, 2018
Copy link
Member

@rshriram rshriram left a comment

Choose a reason for hiding this comment

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

/lgtm

@istio-testing
Copy link
Collaborator

New changes are detected. LGTM label has been removed.

@istio-testing istio-testing removed the needs-rebase Indicates a PR needs to be rebased before being merged label Oct 30, 2018
@hzxuzhonghu
Copy link
Member Author

/test e2e-mixer-no_auth

@hzxuzhonghu
Copy link
Member Author

/retest

@hzxuzhonghu
Copy link
Member Author

/test e2e-bookInfoTests-envoyv2-v1alpha3

@hzxuzhonghu hzxuzhonghu force-pushed the proto-value branch 2 times, most recently from 9a116b3 to bca1d05 Compare November 7, 2018 08:00
@istio-testing
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hzxuzhonghu, rshriram

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

@hzxuzhonghu
Copy link
Member Author

What's wrong with circle ci?

@istio-testing
Copy link
Collaborator

@hzxuzhonghu: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
prow/e2e-simpleTests-cni.sh 4e59444 link /test e2e-simpleTests-cni
prow/istio-integ-k8s-tests.sh 4e59444 link /test istio-integ-k8s-tests
prow/e2e-mixer-no_auth-mcp.sh 4e59444 link /test e2e-mixer-no_auth-mcp

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. I understand the commands that are listed here.

@hzxuzhonghu hzxuzhonghu deleted the proto-value branch November 8, 2018 06:31
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.

None yet

5 participants