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

Add overwrite option to field #3146

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@dlibanori
Copy link
Contributor

dlibanori commented Jul 10, 2013

Add overwrite option to field, when it is true, it will not check for duplicate field definition.

[ fix #3140 ]

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jul 10, 2013

Coverage Status

Coverage increased (+0%) when pulling 451f1da on dlibanori:issue-3140 into d52c33b on mongoid:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jul 10, 2013

Coverage Status

Coverage increased (+0%) when pulling 451f1da on dlibanori:issue-3140 into d52c33b on mongoid:master.

@arthurnn

This comment has been minimized.

Copy link
Contributor

arthurnn commented Jul 10, 2013

Thanks for the PR.
Instead of passing an extra option, should we just ignore the check when the field is _type ? I dunno if I like having an extra option on the field, only for this.

@dlibanori

This comment has been minimized.

Copy link
Contributor

dlibanori commented Jul 10, 2013

That was my first thought, but I think an option is more flexible. With an option you can use duplicate_fields_exception and may force a field duplication.

E.g.:

  class Accuracy
    include Mongoid::Document
    field :precision, type: Integer
  end

  class HighAccuracy < Accuracy
    field :precision, type: Float, duplicate: true
  end

But I really don't like its name: duplicate. 😞

Actually, this is also a kind of improvement for duplicate fields exception.

@arthurnn

This comment has been minimized.

Copy link
Contributor

arthurnn commented Jul 10, 2013

maybe overwrite: true would be a better name? or force_overwrite: true

@dlibanori

This comment has been minimized.

Copy link
Contributor

dlibanori commented Jul 10, 2013

I like 'overwrite'. I will rename it.

@arthurnn

This comment has been minimized.

Copy link
Contributor

arthurnn commented Jul 10, 2013

( can u also squash your commits in one? )

@dlibanori

This comment has been minimized.

Copy link
Contributor

dlibanori commented Jul 10, 2013

Done.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jul 10, 2013

Coverage Status

Coverage decreased (-0%) when pulling 3784b09 on dlibanori:issue-3140 into a8f66d8 on mongoid:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jul 10, 2013

Coverage Status

Coverage remained the same when pulling 3784b09 on dlibanori:issue-3140 into a8f66d8 on mongoid:master.

@arthurnn

This comment has been minimized.

Copy link
Contributor

arthurnn commented Jul 11, 2013

👍 on my side... @durran whats your opinion on this?

Add overwrite option to field definition
When overwrite is true, you may redefine a field without warning or exception
(in case duplicate_fields_exception is enabled).

[ fix #3140 ]
@coveralls

This comment has been minimized.

Copy link

coveralls commented Jul 11, 2013

Coverage Status

Coverage remained the same when pulling 564cc27 on dlibanori:issue-3140 into a8f66d8 on mongoid:master.

@arthurnn

This comment has been minimized.

Copy link
Contributor

arthurnn commented Jul 16, 2013

merged in via mongoid@1a04acd

thanks a lot

@arthurnn arthurnn closed this Jul 16, 2013

@dlibanori dlibanori deleted the dlibanori:issue-3140 branch Jul 17, 2013

@durran durran added the fixed label May 29, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment