Skip to content

Conversation

@frank-zsy
Copy link
Contributor

@frank-zsy frank-zsy commented Jul 12, 2018

pflag.StringSliceVar will split the value into string array with
comma, instead pflag.StringArrayVar will pass the value as a whole
string, so we should use StringArrayVar if we intend to pass the
value into other component.

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

fix #1715

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

pflag.StringSliceVar will split the value into string array with
comma, instead pflag.StringArrayVar will pass the value as a whole
string, so we should use StringArrayVar if we intend to pass the
value into other component.
@frank-zsy frank-zsy requested a review from fuweid July 12, 2018 13:13
@pouchrobot pouchrobot added areas/log kind/bug This is bug report for project size/XS labels Jul 12, 2018
@pouchrobot
Copy link
Collaborator

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

@frank-zsy frank-zsy changed the title bugfix: Fix log-opt option parse fails if value contains comma, because bugfix: Fix log-opt option parse fails if value contains comma Jul 12, 2018
@codecov-io
Copy link

codecov-io commented Jul 12, 2018

Codecov Report

Merging #1729 into master will increase coverage by 0.07%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1729      +/-   ##
==========================================
+ Coverage   41.32%   41.39%   +0.07%     
==========================================
  Files         277      277              
  Lines       18209    18209              
==========================================
+ Hits         7524     7537      +13     
+ Misses       9754     9743      -11     
+ Partials      931      929       -2
Impacted Files Coverage Δ
cli/common_flags.go 0% <0%> (ø) ⬆️
main.go 63.15% <100%> (ø) ⬆️
daemon/logger/jsonfile/utils.go 73.17% <0%> (+1.62%) ⬆️
ctrd/image.go 81.18% <0%> (+4.45%) ⬆️
apis/server/utils.go 66.66% <0%> (+4.76%) ⬆️

@fuweid
Copy link
Contributor

fuweid commented Jul 12, 2018

LGTM

1 similar comment
@fuweid
Copy link
Contributor

fuweid commented Jul 12, 2018

LGTM

@fuweid fuweid merged commit fde6c2a into AliyunContainerService:master Jul 12, 2018
@fuweid
Copy link
Contributor

fuweid commented Jul 12, 2018

close #1715

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

Labels

areas/log kind/bug This is bug report for project size/XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] pouch cli and daemon cannot parse log-opt if the value contains comma

4 participants