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

bind.go: add BindHeaders() to Bind() #2258

Closed
wants to merge 1 commit into from

Conversation

chiyutianyi
Copy link

Headers are supported in Bind() and we can find the following in the
documentation[1]:

Echo provides the following methods to bind data from different
sources to Go Structs using the Context#Bind(i interface{}) method:
- URL Path parameter
- URL Query parameter
- Request body
- Header
  1. https://echo.labstack.com/guide/binding/#bind-using-struct-tags

Headers are supported in Bind() and we can find the following in the
documentation[1]:

	Echo provides the following methods to bind data from different
	sources to Go Structs using the Context#Bind(i interface{}) method:
	- URL Path parameter
	- URL Query parameter
	- Request body
	- Header

1. https://echo.labstack.com/guide/binding/#bind-using-struct-tags

Signed-off-by: Han Xin <hanxin.hx@bytedance.com>
@aldas
Copy link
Contributor

aldas commented Sep 1, 2022

There was a reason why header binding was not added to Bind method. See #1866 (comment)

@chiyutianyi
Copy link
Author

chiyutianyi commented Sep 2, 2022

There was a reason why header binding was not added to Bind method. See #1866 (comment)

Hmm, so should we revise the documentation to keep it the same with the implementation ?

@lammel
Copy link
Contributor

lammel commented Nov 30, 2022

There was a reason why header binding was not added to Bind method. See #1866 (comment)

Hmm, so should we revise the documentation to keep it the same with the implementation ?

@chiyutianyi The section you are referring to is an introduction to the topic, which is then detailed later.
But improving the docs is of course great! You can add a PR for the docs in the https://github.com/labstack/echox project

@derekm
Copy link

derekm commented Oct 12, 2023

@lammel @chiyutianyi -- Can we change this PR to expose BindHeaders() on echo.Context such that I can do:
c.BindHeaders(params)
instead of:
(&echo.DefaultBinder{}).BindHeaders(*c, params)

Thanks!

@aldas
Copy link
Contributor

aldas commented Oct 12, 2023

@derekm we can not change context interface by adding new public method as it would be API breaking change.

@derekm
Copy link

derekm commented Oct 12, 2023

@aldas -- Ahh, right. No default implementations in Go? At least generics are improving! ;) I do hide the ugly code under some reasonable abstractions. Thanks!

@aldas
Copy link
Contributor

aldas commented Oct 12, 2023

we are using semantic versioning and adding new method to interface would be breaking (backwards incompatible) change and would require bumping major version of library to v5. We do not want to do that - not yet.

@aldas aldas closed this Mar 6, 2024
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.

4 participants