EmbeddedDocument should not have save method #1552 #1570
Conversation
|
Looks good to me |
This comment has been minimized.
This comment has been minimized.
|
Totally in favor of removing this as well but I've seen references to this in the issues so be careful as it seems that people started to rely on this... How do you usually document breaking changes? |
|
@erdenezul I really want to merge it but I'm afraid many people will hit this... What we could do before merging this is add a Deprecation warning for a few month. What do you think? |
|
Agree with you @bagerard |
|
alright I'll push to PR that adds the deprecation warning then |
|
@bagerard what about this now? |
|
Is there a way to release the deprecation warnings as 0.16.4 (without the other commits from master as there are some minor breaking changes)? so that we can merge this PR as part of the next 0.17.0 (I guess it will be released in a few months) and hope that people faced the deprecation warning in the meantime |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@bagerard Please review sir. |
wojcikstefan
left a comment
wojcikstefan
left a comment
There was a problem hiding this comment.
LGTM 👌 The last thing I'd ask for is to add this breaking change to docs/changelog.rst.
|
@wojcikstefan Thanks for the review guys. I've added changelog @bagerard |
5 participants
Closes #1552