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

fix client cannot unmarshal struct which contains null #1788

Merged
merged 6 commits into from
Nov 15, 2018

Conversation

alexz0000
Copy link

@alexz0000 alexz0000 commented Nov 9, 2018

fixes #1729

@casualjim
Copy link
Member

Can you run: go generate ./generator . that way bindata.go will get updated
We use this bindata: https://github.com/kevinburke/go-bindata

Can you also sign off on your commit? https://github.com/go-swagger/go-swagger/pull/1788/checks?check_run_id=30358019

Signed-off-by: alex.chen <alex.chen@logicmonitor.com>
@codecov
Copy link

codecov bot commented Nov 9, 2018

Codecov Report

Merging #1788 into master will increase coverage by 0.1%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #1788     +/-   ##
=========================================
+ Coverage   80.04%   80.14%   +0.1%     
=========================================
  Files          38       38             
  Lines        7497     7485     -12     
=========================================
- Hits         6001     5999      -2     
+ Misses       1019     1008     -11     
- Partials      477      478      +1
Impacted Files Coverage Δ
generator/bindata.go 68.34% <100%> (+1.61%) ⬆️
generator/template_repo.go 87.43% <0%> (-0.97%) ⬇️

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 800a305...18394a2. Read the comment docs.

@casualjim
Copy link
Member

Can you please run: go generate ./generator?
That way generator/bindata.go will get updated, and your changes will actually get picked up.
We use this bindata: https://github.com/kevinburke/go-bindata

Signed-off-by: alex.chen <alex.chen@logicmonitor.com>
@alexz0000
Copy link
Author

Done, thanks

@alexz0000
Copy link
Author

Hi @casualjim, Does the failed test cases means I don't have the permission to update the template?

@casualjim
Copy link
Member

I think it means that the assertion is for propNodes, err := Unmarshal... but the code has nodes, err := Unmarshal...

@fredbi
Copy link
Contributor

fredbi commented Nov 13, 2018

the prop prefix was added to avoid naming conflicts

@alexz0000
Copy link
Author

the nodes is just a tmp variable, I think it's OK here to use nodes, err := Unmarshal..., what do you think?

@casualjim
Copy link
Member

I think irrespective of the naming discussion, the test isn't passing, so the test needs an updated assertion. You can run the tests with go test ./generator/...

@fredbi
Copy link
Contributor

fredbi commented Nov 14, 2018

the nodes is just a tmp variable, I think it's OK here to use nodes, err := Unmarshal..., what do you think?

Don't. How about a field named err?

alex.chen added 4 commits November 15, 2018 15:20
Signed-off-by: alex.chen <alex.chen@logicmonitor.com>
Signed-off-by: alex.chen <alex.chen@logicmonitor.com>
Signed-off-by: alex.chen <alex.chen@logicmonitor.com>
Signed-off-by: alex.chen <alex.chen@logicmonitor.com>
@alexz0000
Copy link
Author

I updated the cases, could you help to review and merge the code ?

@casualjim casualjim merged commit cd1d9ec into go-swagger:master Nov 15, 2018
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.

Client cannot unmarshal struct
3 participants