mongoid
mongoid copied to clipboard
mongodb
[READY FOR REVIEW] MONGOID-5336: User-defined symbol field types
@p-mongo I believe I have resolved all your requested changes. Please review again, thanks.
Have done all requested second round changes. Appreciate if you can resolve the comments once you check them.
I like the block configuration syntax but it should be on top-level Mongoid module, e.g. Mongoid.configure do ... not nested under fields.
The actual changes pertaining to alias definition I think are mostly OK.
I'm not seeing the purpose of maintaining two mappings - the "default" Mongoid one and the active one. Can you explain this part please?
I like the block configuration syntax but it should be on top-level Mongoid module.
Sure, will be glad to do this change.
I'm not seeing the purpose of maintaining two mappings - the "default" Mongoid one and the active one. Can you explain this part please?
Mongoid::Fields::FieldTypes::DEFAULT_MAPPING is an immutable constant. The active mapping Mongoid::Fields::FieldTypes.mapping (singleton) is initialized to a dup of this constant, and is mutable so users can add their own types. It would of course be possible to inline the constant into the .mapping method, but I thought it was cleaner to keep it separate (plus we need it for the deprecated Mongoid::Fields::TYPE_MAPPINGS constant.)
Does this answer your question?
@p-mongo config change done, ready for a final review.
Note the config will look like this:
Mongoid.configure do |config|
config.define_field_type :point, Point
end
To keep consistency, I'm also adding config.define_field_option in this PR.
I've raised https://jira.mongodb.org/browse/MONGOID-5422 (PR https://github.com/mongodb/mongoid/pull/5367) to support doing configure without the |config| arg. It can be done in a backwards compatible manner.
Please kindly remove the "Oleg Responded" / "Oleg Todo" flags on this and all other PRs.