Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

added .rvmrc to gitignore, fixed a few syntax typos #9

Merged
merged 6 commits into from Apr 10, 2012

Conversation

Projects
None yet
3 participants

paxer commented Mar 27, 2012

No description provided.

Owner

Draiken commented Mar 27, 2012

Thank you very much! I18n was long overdue...

I'll check this as soon as possible and I'll merge and issue a new gem version.

Thanks again!

Pavel Kotlyar added some commits Mar 28, 2012

paxer commented Mar 28, 2012

no probs at all, it's fun :)
One more thing I am going to add is - move comments "order_by" to the config (similar to what I've done with strip_html_tags_on_save).

What do you think ? How to do it better ?

The test is not gonna pass because the check for strip_html_tags_on_save is made when the opinio is loaded. so even tho you change the option on the setup, the before_save was already called.

Owner

Draiken commented Mar 28, 2012

I think it's good this way.

I thought about removing it from opinio, and letting the user set, for example, a default scope on the comment model
but I really don't like default scopes, so I think that's the best way to do it.

Since you're adding another configuration option, you can add it on the generator template for the opinio initializer so that people know they can change that too.

paxer commented Mar 28, 2012

yes, I've added "strip_html_tags_on_save" as well as "sort_order" to the generator template with default values.
Regarding the test - that's what I thought too. Any ideas how to make it pass ?

Owner

Draiken commented Mar 29, 2012

I really don't know how we can make that test pass...

The behaviour is correct, we don't need to check if the flag is on at runtime :/
I'll try to think of something.

Owner

Draiken commented Apr 10, 2012

I'm gonna fix that test as soon as I can. Gonna pull this in for now

Draiken added a commit that referenced this pull request Apr 10, 2012

Merge pull request #9 from paxer/master
Fixing JS typos. Adding i18n and other options

@Draiken Draiken merged commit 4947fb0 into Draiken:master Apr 10, 2012

fred commented Apr 19, 2012

Cool, thanks for this Pull.
I had done very similar customizations to the views, now it's official.
cheers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment