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

Adds description about various options available with the acts_as_list method #168

Merged
merged 1 commit into from
Sep 7, 2015

Conversation

udit7590
Copy link

No description provided.

@@ -85,6 +85,15 @@ class TodoItem < ActiveRecord::Base
end
```

## More Options
You can pass the following options with **acts_as_list** class method
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for this line.

@swanandp
Copy link
Contributor

This documentation is best left to the code. By putting it in README, we're only repeating ourselves.

@udit7590
Copy link
Author

I think README file is to help developers understand how to use the gem.
I faced an issue where I was not able to determine what is the default value that is being picked up by position cloumn when I was saving my records. When I finally looked at the code, I realized that I can actually use the option add_new_at available with the acts_as_list method. If it would have been present in README file it would have saved a lot of time for me.
Hence I believe README file should tell about all the behavior and options available with the gem to help developers understand how wisely they could use the gem.

@brendon
Copy link
Owner

brendon commented Sep 2, 2015

I kind of agree with @udit7590, it is handy to have the options documented in the README. Up to you though Swanand. :)

@akshay-vishnoi
Copy link

👍

@swanandp
Copy link
Contributor

swanandp commented Sep 7, 2015

Alright, let's put it in. @udit7590 Can you make the fix I commented on, and then squash the two commits? I am generally not in favour of squashing, but the commits here are tiny enough to warrant a squash.

@udit7590 udit7590 force-pushed the explain-all-options branch from 620ecd5 to 6ccb97e Compare September 7, 2015 08:46
@udit7590
Copy link
Author

udit7590 commented Sep 7, 2015

@swanandp Squashed and fixed the changes.

@swanandp
Copy link
Contributor

swanandp commented Sep 7, 2015

Alright, merging this in.

swanandp added a commit that referenced this pull request Sep 7, 2015
Adds description about various options available with the acts_as_list method
@swanandp swanandp merged commit e14d4fb into brendon:master Sep 7, 2015
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.

5 participants