search

noir-clojure / lib-noir

A set of libraries for ring apps, including stateful sessions.
Eclipse Public License 1.0
483 stars 47 forks source link

It's not obvious that `noir.validation.rule` updates an atom #96

Closed john2x closed 10 years ago

john2x commented 10 years ago

Maybe the docs could be improved? Mention in wrap-noir-validation that it adds an *errors* binding of an atom, and it gets populated by the various functions provided by the validation namespace. Right now, unless you read the source, it's not clear where the errors are stored.

And/or maybe add the ! suffix to rule, set-error and clear-errors? But that would break the API.

yogthos commented 10 years ago

That's definitely a good point. Originally, these were kept as is not to introduce breaking changes. It might be an idea to mark the current functions that modify atoms as deprecated and create new ones using the standard ! postfix.

Updating the documentation is definitely a good idea as well.

john2x commented 10 years ago

I could do the docs if you want. I've been drafting a blog post about a walkthrough with Luminus, and I just finished the part where I explain a bit about the validation namespace.

yogthos commented 10 years ago

I'll definitely update the docs for now, but this is something I wanted to clean up for a little while. Right now the API is breaking the standard convention, which isn't good. :)

john2x commented 10 years ago

Cool, thanks! Should this be closed or...?

yogthos commented 10 years ago

Let's wait till I make the updates so I don't forget about it. :)

yogthos commented 10 years ago

ok new version of lib-noir is up, I updated the docs and released a new luminus template with the latest version

yogthos commented 10 years ago

@john2x btw I added some docs in the luminus site http://www.luminusweb.net/docs/input_validation.md let me know if it all makes sense :)

john2x commented 10 years ago

Looks great :)