Namespace config #53
Namespace config #53
Conversation
jarthod
left a comment
jarthod
left a comment
There was a problem hiding this comment.
Thanks for this MR, I like the idea, a couple comments
| class TestCase < Minitest::Test #:nodoc: | ||
|
|
||
| def setup | ||
| Mongoid::Migrator.migrations_path = ["db/migrate"] |
| assert_equal ["db/mongoid"], Mongoid::Migrator.migrations_path | ||
| Mongoid::Migrator.migrations_path += ["engines/my_engine/db/migrate"] | ||
| assert_equal ["db/mongoid", "engines/my_engine/db/migrate"], Mongoid::Migrator.migrations_path | ||
| end |
There was a problem hiding this comment.
This is not really testing the namespacing of tasks ^^
There was a problem hiding this comment.
It doesn't but practically speaking if one really wants to de-conflict with ActiveRecord they should be modifying both the namespace and the path. Coz if the actual migration files of AR and Mongo live in the same path the rake tasks will still fail. Hence I tested both in the same test case.
Speaking it out loud, this caveat should be mentioned in the README file too
There was a problem hiding this comment.
Sure, no problem with that, what I mean is that the namespacing itself is not tested, if you remove all the changes in database.rake this spec still pass isn't it? I'm not sure if it's easy to do so but I think we should verify the list of rake tasks generated, that their name is correct.
|
Thanks so much! This would close #59 |
|
@jgandt-wp Indeed, if you like this approach too, feel free to continue this MR (it's mostly missing specs AFAICS). |
jarthod
force-pushed
the
master
branch
2 times, most recently
from
f4509bc to
3bb749d
Compare
|
@heitor-s indeed nobody finished it yet, feel free to continue and submit it (with specs 🙏) |
4 participants
Make rake task namespace to configurable so conflict with
ActiveRecordrake tasks can be avoided