Skip to content

Conversation

@zhuangqh
Copy link
Contributor

Ⅰ. Describe what this PR did

  1. fix the gocyclo part of goreport
  2. fix bug: the no-proxy's environment name should be 'NO_PROXY' or 'no_proxy', not 'noProxy'.

Ⅱ. Does this pull request fix one issue?

fixes #964

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@pouchrobot pouchrobot added kind/bug This is bug report for project size/M labels Apr 10, 2018
@pouchrobot
Copy link
Collaborator

@zhuangqh Thanks for your contribution. 🍻
Please sign off in each of your commits.

@CLAassistant
Copy link

CLAassistant commented Apr 10, 2018

CLA assistant check
All committers have signed the CLA.

@pouchrobot
Copy link
Collaborator

We found this is your first time to contribute to Pouch, @zhuangqh
👏 We really appreciate it.
Just remind that you have read the contribution guide: https://github.com/alibaba/pouch/blob/master/CONTRIBUTING.md
If you didn't, you should do that first. If done, welcome again and please enjoy hacking! 🍻

@codecov-io
Copy link

Codecov Report

Merging #1084 into master will decrease coverage by 0.05%.
The diff coverage is 90%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1084      +/-   ##
==========================================
- Coverage    16.2%   16.15%   -0.06%     
==========================================
  Files         161      158       -3     
  Lines        8850     8842       -8     
==========================================
- Hits         1434     1428       -6     
+ Misses       7312     7310       -2     
  Partials      104      104
Impacted Files Coverage Δ
ctrd/image_proxy_util.go 0% <ø> (ø) ⬆️
pkg/utils/utils.go 78.15% <90%> (-4.03%) ⬇️
daemon/mgr/image.go 32.12% <0%> (-0.34%) ⬇️
client/container.go 0% <0%> (ø) ⬆️
pkg/errtypes/errors.go 35.29% <0%> (ø) ⬆️
client/container_unpause.go
client/container_pause.go
client/client.go

@allencloud
Copy link
Collaborator

Thanks a lot for your contribution. @zhuangqh
Also cc @zeppp Could you help to review this?

var (
noProxyEnv = &envOnce{
names: []string{"NO_PROXY", "noProxy"},
names: []string{"NO_PROXY", "no_proxy"},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you help to confirm this? Since I think it is introduced by your part. @Ace-Tang

Copy link
Collaborator

Choose a reason for hiding this comment

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

have communicated with @Ace-Tang offline. The change is totally reasonable.

@zeppp
Copy link
Contributor

zeppp commented Apr 10, 2018

LGTM

1 similar comment
@allencloud
Copy link
Collaborator

LGTM

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Apr 11, 2018
@allencloud
Copy link
Collaborator

Thank @zhuangqh for your first contribution.

@allencloud allencloud merged commit a6fdc3a into AliyunContainerService:master Apr 11, 2018
@allencloud
Copy link
Collaborator

allencloud commented Apr 11, 2018

Oh, I have forgotten to squash the four commits into a single one. My fault.
And please submit a pull request with only a single commit in the following pull request. Thanks a lot. @zhuangqh

@zhuangqh
Copy link
Contributor Author

Sorry my fault. I have read the contributing guide carefully. 😅 @allencloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug This is bug report for project LGTM one maintainer or community participant agrees to merge the pull reuqest. size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[GoReport Issue] fix issue for Pouch reported from GoReport

6 participants