Skip to content

Add support for encoding.TextUnmarshaler in bind. #1314

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

Merged
merged 1 commit into from
Jun 9, 2019

Conversation

gdamore
Copy link
Contributor

@gdamore gdamore commented Mar 29, 2019

This permits types that already implement encoding.TextUnmarshaler (such time.Time) to automatically be supported as parameters. Types that implement the BindUnmarshaler interface use that API in preference, but having the fallback greatly reduces the effort in using time.Time in particular, and probably a number of other types besides. This change is contributed by RackTop Systems -- if there is a way to acknowledge that in a copyright statement it should read "Copyright 2019 RackTop Systems".

@codecov
Copy link

codecov bot commented Mar 29, 2019

Codecov Report

Merging #1314 into master will increase coverage by 0.18%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1314      +/-   ##
==========================================
+ Coverage   84.27%   84.45%   +0.18%     
==========================================
  Files          27       27              
  Lines        1990     2001      +11     
==========================================
+ Hits         1677     1690      +13     
+ Misses        205      203       -2     
  Partials      108      108
Impacted Files Coverage Δ
bind.go 90.85% <100%> (+1.83%) ⬆️

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 e3717be...3de4ae4. Read the comment docs.

@stale
Copy link

stale bot commented May 28, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label May 28, 2019
@gdamore
Copy link
Contributor Author

gdamore commented May 28, 2019

Wow. Is this project still alive even?

@stale stale bot removed the wontfix label May 28, 2019
@gdamore
Copy link
Contributor Author

gdamore commented May 28, 2019

As a contributor, let me comment here. We spent effort to produce a fix (which is a real problem for us). I can understand if someone actively chose to reject the PR, or asked us to update to newer code, or make some other changes. But to be marked "stale" and "wontfix" by some bot for inactivity on an otherwise well-formed PR is really really infuriating -- it smacks of complete disrespect to potential contributors.

To be clear, I run many open source projects myself, and I don't always respond to every PR in a timely fashion, and sometimes I reject PRs. But the idea of having a bot go and reap issues that I haven't even bothered to look at myself (or haven't gotten time to look at yet) is one of the most ludicrous policies I've ever heard of.

@vishr vishr merged commit c824b8d into labstack:master Jun 9, 2019
@vishr
Copy link
Member

vishr commented Jun 9, 2019

@gdamore I apologize for the bot. I am trying to be more active now.

@gdamore
Copy link
Contributor Author

gdamore commented Jun 9, 2019

Thanks!

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.

2 participants