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

Many contributed features #10

Open
wants to merge 30 commits into
base: master
Choose a base branch
from
Open

Conversation

fagiani
Copy link

@fagiani fagiani commented Mar 9, 2011

Hi Wilker,

I really enjoyed the effort you put at this and I am also proud that you are a Northeastern Brazilian (I'm from Natal).

I saw that some of the features I intended to contribute were already implemented by others in the project's network so I pretty much merged their work as I need those features too.

My actual contributions are regarding the event callbacks. Please feel free to cherry pick what you like.

Thanks and keep up the great work!

ches and others added 26 commits January 15, 2011 20:50
Since this is map/reduce aggregation, calling it indexing is potentially
confusing.
Field name is now specified when calling the macro, or it defaults to
'tags':

   taggable :keywords

We're no longer using a field name different from the accessors --
when modifying the tags field a string argument will automatically be
converted to an array. We always store an array and always get one back
from the reader. It is the view's (or helper's) job to format tags for
display.

   article.keywords = ['opinion', 'politics']
   article.keywords.should == %w[ opinion politics ]
   article.keywords = 'sports, lacrosse'
   article.keywords.should == %w[ sports lacrosse ]
Mongoid::Config is also no longer a singleton in 2.0.0
Conflicts:
	.gitignore
	Rakefile
	lib/mongoid/taggable.rb
	mongoid_taggable.gemspec
	spec/spec_helper.rb
@ches
Copy link
Contributor

ches commented Mar 10, 2011

Your changes look good to me, cleaned up the callbacks nicely. The only controversy is that I ditched Jeweler for Bundler because I feel Jeweler isn't necessary, but Wilker didn't want to do that. We could rebase the subsequent changes to a branch without that commit and then merge without much trouble I think.

It doesn't make sense to track Gemfile.lock in a library repo, and it can in fact be a hassle -- reasoning is here:

http://yehudakatz.com/2010/12/16/clarifying-the-roles-of-the-gemspec-and-gemfile/

Wilker and I just exchanged messages so maybe together we can sort out getting most of this merged/cherry picked. I was mostly hoping for Mongoid 2.0.0 to be released soon to verify things still work before submitting a pull request for the work I'd done.

@fagiani
Copy link
Author

fagiani commented Mar 10, 2011

Hey Ches,

Thanks for that link, I got the differences very clear now while I am starting to contribute with gem's more deeply as I learn.

I was not aware of the Jewler issue but I agree that we need to work around it and hear Wilker on his guidelines and concerns as the project creator. Let's hope that Mongoid 2 won't break a thing! :)

I really appreciate your refactor and be sure that I've learned lots with that.

andresf referenced this pull request in andresf/mongoid_taggable Apr 22, 2013
@yordis
Copy link

yordis commented Sep 30, 2015

@wilkerlucio please maintain this 😢 this is a common problem for developers

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.

4 participants