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

Params Time Binding #878

Closed
ready4god2513 opened this issue Feb 21, 2015 · 5 comments
Closed

Params Time Binding #878

ready4god2513 opened this issue Feb 21, 2015 · 5 comments
Assignees
Labels
effort-minutes Will take up to 60 minutes to implement priority-must now, top priority status-planning Active planning underway status-tested Ready for release topic-config topic-template
Milestone

Comments

@ready4god2513
Copy link

I may have been doing something wrong, but I believe that there is a mistake in the docs. The docs state that The SQL standard time formats [“2006-01-02”, “2006-01-02 15:04”] are built in. but that doesn't appear to be the case.

Right here is where the bindings are set- https://github.com/revel/revel/blob/master/binder.go#L226 which is perfect. HOWEVER, the default config to ship with revel actually does have the date format set here- https://github.com/revel/revel/blob/master/skeleton/conf/app.conf.template#L69 So by default revel appears to only bind on 01/02/2006 and 01/02/2006 15:04

So either the docs need to change or the default config needs to change.

Let me know if I am incorrect on this. Thanks!

@notzippy notzippy added status-planning Active planning underway topic-template topic-config status-tested Ready for release labels Feb 21, 2015
@ghost
Copy link

ghost commented Feb 21, 2015

Hi, @ready4god2513. You are right, looks like the docs are outdated.

The SQL standard time formats [“2006-01-02”, “2006-01-02 15:04”] are built in.

These formats were built in ages ago:
SQL time formats
Now they are not.

Fix is more than welcome here: https://github.com/revel/revel.github.io/tree/develop

Related docs links

https://revel.github.io/manual/appconf.html#formatting
https://revel.github.io/manual/binding.html#date--time

@ghost ghost mentioned this issue Feb 22, 2015
6 tasks
@ghost
Copy link

ghost commented Feb 22, 2015

We are probably coming back to the time format mentioned in the docs: #872 (comment). So, changes to the docs may not be needed.

@brendensoares brendensoares added effort-minutes Will take up to 60 minutes to implement priority-must now, top priority labels Mar 16, 2015
@brendensoares brendensoares added this to the v0.13 milestone Mar 16, 2015
@ptman
Copy link
Contributor

ptman commented May 31, 2016

So change config to ISO8601 ? I can do a PR if that moves this forward.

@jeevatkm
Copy link
Contributor

As per my understanding, binder code look okay. Just skeleton app.conf needs an update from

format.date     = 01/02/2006
format.datetime = 01/02/2006 15:04

to

format.date     = 2006-01-02
format.datetime = 2006-01-02 15:04

@ptman would you like to do it?

jeevatkm added a commit that referenced this issue Jun 1, 2016
Fix skeleton config to match code and doc. #878
@jeevatkm jeevatkm self-assigned this Jun 4, 2016
@jeevatkm
Copy link
Contributor

jeevatkm commented Jun 4, 2016

Taken care, thanks to @ptman

I'm closing this one.

@jeevatkm jeevatkm closed this as completed Jun 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort-minutes Will take up to 60 minutes to implement priority-must now, top priority status-planning Active planning underway status-tested Ready for release topic-config topic-template
Projects
None yet
Development

No branches or pull requests

5 participants