这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@deo986
Copy link
Collaborator

@deo986 deo986 commented Apr 22, 2022

Today in order to use a boolean with queryparams it needs to be set like this: ?param=true, now you can set it with ?param or ?param

@codecov
Copy link

codecov bot commented Apr 22, 2022

Codecov Report

Merging #44 (2a2fc53) into main (49db25b) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #44      +/-   ##
==========================================
+ Coverage   86.63%   86.64%   +0.01%     
==========================================
  Files          24       24              
  Lines        1930     1932       +2     
==========================================
+ Hits         1672     1674       +2     
  Misses        183      183              
  Partials       75       75              
Impacted Files Coverage Δ
resolver.go 86.16% <100.00%> (+0.11%) ⬆️

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 49db25b...2a2fc53. Read the comment docs.

resolver.go Outdated
Comment on lines 278 to 280
} else if req.URL.Query().Has(name) {
// name has no associated value, but exists in the map of QueryParams. This is a boolean value
pv = "true"
Copy link
Owner

Choose a reason for hiding this comment

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

I think we also need to check that the type is a boolean here, otherwise for your example tests below I think going GET /?s=&b will actually set the string to "true" rather than "". It's an edge case for sure, but probably one we want to avoid if possible.

Copy link
Owner

@danielgtaylor danielgtaylor left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@deo986 deo986 merged commit c194251 into main Apr 22, 2022
@deo986 deo986 deleted the query-params branch April 22, 2022 20:50
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.

3 participants