cviebrock / eloquent-taggable

Easily add the ability to tag your Eloquent models in Laravel.
MIT License
559 stars 72 forks source link

adding support for alternate database connection #46

Closed swatkins closed 7 years ago

cviebrock commented 7 years ago

Great idea ... thank you! A few comments:

  1. I'm going to rename the configuration flag from db_connection to just connection? I think it's obvious enough.
  2. Not sure why all the tests are failing. I don't think it has anything to do with your code, so I'll look into that after merging.
  3. Next time, if you can add a test and documentation for your change, that would be appreciated. I'll take care of it for now.

Thanks again!

cviebrock commented 7 years ago

FWIW, I can't imagine the code you submitted actually worked on your end. The Tag model's new constructor didn't have the right arguments, nor did it call the parent constructor.

The new version I'm pushing shortly fixes all that.

swatkins commented 7 years ago

You're right, I missed that. I have done all these changes within the vendor folder of my project then tried to pour them over to this pool request. Apparently I missed it.