-
Notifications
You must be signed in to change notification settings - Fork 11
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
Issue25 #137
base: master
Are you sure you want to change the base?
Issue25 #137
Conversation
test "the truth" do | ||
assert true | ||
end | ||
module DiscussionsHelper | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I swear git is crazy sometimes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's wrong?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh
nvm.....
There's a lot here, so I can't be sure you did everything but once you do what I suggested you should be ready to merge. |
@@ -23,7 +23,7 @@ def setup_variables(comment) | |||
@commentable = @comment_thread.commentable.becomes( | |||
Kernel.const_get(@comment_thread.commentable_type)) | |||
@active_subscribers = User.subscribers_for(@comment_thread).active_users | |||
@is_message = @comment_thread.commentable_type == 'Message' | |||
@is_message = @comment_thread.commentable_type == 'Discussion' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well just a minor thing you might wanna rename this to @is_discussion (and same on the method above)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ooops, I seemed to have missed that
@Dantemss - status? |
@jpslav Should be good but once again I suggest not merging schema.rb if you can. |
will push the correction to the schema |
@Dantemss how does this pull request jive with a future that uses commontator? |
Adding commontator to Quadbase would require many changes, including getting rid of this. However, I didn't plan on doing that for now. I would have to update commontator to add support for 'Discussions' and migrate the comments/votes tables to commontator's. |
Don't worry about it for now. On Sep 13, 2012, at 9:52 AM, Dantemss wrote:
|
solves issue #25
changed everything from Message to Discussion.
There is a change in the migrate folder that renames the messages_table to discussions_table
When pulling this branch in, please rake db:migrate to make the change to the table