SWIFT-630 Integrate the BSON enum into the public API #336
Conversation
kmahar
left a comment
kmahar
left a comment
There was a problem hiding this comment.
this looks great, and all the stuff you were able to rewrite / condense seems like further confirmation to me that this new API is the right choice. my comments are mostly nits and a few questions.
| /// Decode a BSONValue type from this container for the given key. | ||
| private func decodeBSONType<T: BSONValue>(_ type: T.Type, forKey key: Key) throws -> T { | ||
| let entry = try getValue(forKey: key) | ||
| return try self.decoder.with(pushedKey: key) { |
There was a problem hiding this comment.
is still with(pushedKey:) thing still called on the new code paths that used to use this?
There was a problem hiding this comment.
it's still called in decodeNumber and decode(type:forKey).
There was a problem hiding this comment.
what if you are decoding e.g. a Bool or a String (line 582, 597)? it seems to me like those can get called directly now without adding the key that is being decoded to the key path
There was a problem hiding this comment.
Ohh, I misunderstood what you were saying. Yeah that's a very good point; I shouldn't be using an "unboxing" method to do "decoding". I've gone ahead and reverted removing this method and made the two examples you mentioned call it again.
patrickfreed
force-pushed
the
SWIFT-630/integrate-bson
branch
from
984c51a to
81f223d
Compare
SWIFT-630
This PR updates the public API and tests to use the new
BSONenum. As part of this, the following changes have been made:BSONValueprotocol is now internalArrayconditionally conforms toBSONValuewhenElement == BSONAnyBSONValuehas been removedbsonEqualsand its associated Nimble matcher have been removedMost of the implementation of the BSON library has remained the same with the exception of the encoder/decoder, which required some changes.