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

refactor(form_mapping.go): mapping ptr, struct and map #1749

Merged
merged 10 commits into from
Mar 3, 2019

Conversation

vkd
Copy link
Contributor

@vkd vkd commented Jan 22, 2019

Run benchmark:
go test -run=NONE -benchmem -bench=BenchmarkMapForm ./binding -benchtime=20s > new.bench

Compare benchcmp old.bench new.bench:

benchmark                  old ns/op     new ns/op     delta
BenchmarkMapFormFull-4     2706          2476          -8.50%
BenchmarkMapFormName-4     271           228           -15.87%

benchmark                  old allocs     new allocs     delta
BenchmarkMapFormFull-4     22             12             -45.45%
BenchmarkMapFormName-4     3              1              -66.67%

benchmark                  old bytes     new bytes     delta
BenchmarkMapFormFull-4     384           200           -47.92%
BenchmarkMapFormName-4     40            8             -80.00%

* fix gin-gonic#1672 correct work with ptr - not create value if field is not set
* avoid allocations on strings.Split() - change to strings.Index()
* fix gin-gonic#610 tag value "-" is mean ignoring field
* struct fields mapped like json.Unmarshal
* map fields mapped like json.Unmarshal
@codecov
Copy link

codecov bot commented Jan 22, 2019

Codecov Report

Merging #1749 into master will increase coverage by 0.11%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1749      +/-   ##
==========================================
+ Coverage    98.5%   98.62%   +0.11%     
==========================================
  Files          41       41              
  Lines        2073     2103      +30     
==========================================
+ Hits         2042     2074      +32     
+ Misses         19       18       -1     
+ Partials       12       11       -1
Impacted Files Coverage Δ
binding/form_mapping.go 100% <100%> (ø) ⬆️
binding/form.go 100% <0%> (+7.4%) ⬆️

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 893c6ca...9c8c90d. Read the comment docs.

binding/binding_test.go Outdated Show resolved Hide resolved
binding/binding_test.go Outdated Show resolved Hide resolved
binding/binding_test.go Outdated Show resolved Hide resolved
binding/binding_test.go Outdated Show resolved Hide resolved
binding/form_mapping.go Outdated Show resolved Hide resolved
binding/form_mapping.go Outdated Show resolved Hide resolved
binding/form_mapping.go Outdated Show resolved Hide resolved
binding/form_mapping.go Outdated Show resolved Hide resolved
thinkerou
thinkerou previously approved these changes Feb 20, 2019
@thinkerou thinkerou removed this from the 1.4 milestone Mar 1, 2019
@thinkerou thinkerou added this to the 1.5 milestone Mar 1, 2019
Copy link
Member

@thinkerou thinkerou left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@thinkerou
Copy link
Member

@appleboy please review the pull request, thanks!

@thinkerou thinkerou modified the milestones: 1.5, 1.4 Mar 2, 2019
@thinkerou thinkerou merged commit 0d50ce8 into gin-gonic:master Mar 3, 2019
thinkerou added a commit to gin-gonic/website that referenced this pull request Mar 4, 2019
@vkd vkd deleted the new_mapping branch March 26, 2019 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants