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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 10 commits into from Mar 3, 2019
Merged

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

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%

@vkd
refactor(form_mapping.go): mapping ptr, struct and map
ea237a8 
* 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 •
edited

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.

@thinkerou thinkerou added this to the 1.4 milestone Jan 24, 2019
thinkerou
thinkerou reviewed Jan 24, 2019
View reviewed changes
binding/binding_test.go Outdated Show resolved Hide resolved
thinkerou
thinkerou reviewed Jan 24, 2019
View reviewed changes
binding/binding_test.go Outdated Show resolved Hide resolved
thinkerou
thinkerou reviewed Jan 24, 2019
View reviewed changes
binding/binding_test.go Outdated Show resolved Hide resolved
thinkerou
thinkerou reviewed Jan 24, 2019
View reviewed changes
binding/binding_test.go Outdated Show resolved Hide resolved
thinkerou
thinkerou reviewed Jan 24, 2019
View reviewed changes
binding/form_mapping.go Outdated Show resolved Hide resolved
thinkerou
thinkerou reviewed Jan 24, 2019
View reviewed changes
binding/form_mapping.go Outdated Show resolved Hide resolved
thinkerou
thinkerou reviewed Jan 24, 2019
View reviewed changes
binding/form_mapping.go Outdated Show resolved Hide resolved
thinkerou
thinkerou reviewed Jan 24, 2019
View reviewed changes
binding/form_mapping.go Outdated Show resolved Hide resolved
thinkerou
thinkerou previously approved these changes Feb 20, 2019
View reviewed changes
@vkd vkd mentioned this pull request Feb 21, 2019
Closed
@thinkerou thinkerou removed this from the 1.4 milestone Mar 1, 2019
@thinkerou thinkerou added this to the 1.5 milestone Mar 1, 2019
thinkerou
thinkerou approved these changes Mar 2, 2019
View reviewed changes
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
This was referenced Mar 3, 2019
Closed
Closed
thinkerou added a commit to gin-gonic/website that referenced this pull request Mar 4, 2019
@thinkerou
sync gin #1749 update
aeefbe6 
details see gin-gonic/gin#1749
@vkd vkd deleted the new_mapping branch March 26, 2019 18:29
@omkar-foss omkar-foss mentioned this pull request Jul 10, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thinkerou thinkerou thinkerou approved these changes

@appleboy appleboy Awaiting requested review from appleboy

@javierprovecho javierprovecho Awaiting requested review from javierprovecho

Assignees
No one assigned
Labels
enhancement feature
Projects
None yet
Milestone
1.4
3 participants
@vkd @thinkerou @appleboy