Skip to content
This repository was archived by the owner on Dec 23, 2024. It is now read-only.

query param access token#24

Merged
talpert merged 3 commits into
masterfrom
qparamtoken
Mar 22, 2017
Merged

query param access token#24
talpert merged 3 commits into
masterfrom
qparamtoken

Conversation

@talpert
Copy link
Copy Markdown
Contributor

@talpert talpert commented Mar 21, 2017

option to read the access token from a query param

@codecov-io
Copy link
Copy Markdown

codecov-io commented Mar 21, 2017

Codecov Report

Merging #24 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master    #24   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           8      8           
  Lines         177    193   +16     
=====================================
+ Hits          177    193   +16
Impacted Files Coverage Δ
middleware_accesstoken.go 100% <100%> (ø) ⬆️

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 e4ee692...139d65e. Read the comment docs.

Copy link
Copy Markdown
Contributor

@dselans dselans left a comment

Choose a reason for hiding this comment

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

Leaving up to you to decide whether you update to switch or not.

Comment thread middleware_accesstoken.go Outdated
tokens: tokens,
}

if tokenType == "query" {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think a switch statement would be nice here - it's easier to read and see what's happening (and you can also set the default case to err if the tokenType is not supported (to prevent future, potential errors)).

@talpert talpert merged commit 759e085 into master Mar 22, 2017
@talpert talpert deleted the qparamtoken branch March 22, 2017 17:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants