Update doc comments, add PropertyWrappersTest
#246
Conversation
|
Hi @CoreOffice/xmlcoder-team, would you have a moment to take a look? I wanted to make sure we're on the same page in terms of documentation, so your PR review here would be appreciated. Thanks! |
| case element | ||
| /// Decodes a node from either elements of form `<nodeName>value</nodeName>` or attributes | ||
| //// of form `nodeName="value"`. |
There was a problem hiding this comment.
Do we guarantee the order in which the check is made?
There was a problem hiding this comment.
Great catch, I've clarified it in the comment
|
|
||
| func testDecode() throws { | ||
| let decoder = XMLDecoder() | ||
| let decodedBookBoth = try decoder.decode(Book.self, from: Data(bookAuthorElementAndAttributeXML.utf8)) |
There was a problem hiding this comment.
Shouldn’t we also add a from string decoding API, if one is seemingly lacking. That is not a change request, but a question
There was a problem hiding this comment.
I think this would be a sensible addition and raises a question why JSONDecoder haven't got anything like this by now? XMLCoder was modelled after JSONDecoder and JSONEncoder APIs.
|
What are the odds that people currently rely on this API? I’m not exactly for a breaking change, but I wonder what the impact would be |
If you're referring to In theory, I could expect someone conforming to this protocol similarly by accident, but even doing that would require some conscious effort on their side, and they shouldn't be doing this in the first place. Since XMLCoder 1.0 haven't been tagged yet so far, I think it's acceptable to introduce this breaking change for the sake of having a cleaner API. |
When viewing documentation published at https://swiftpackageindex.com/CoreOffice/XMLCoder/main/documentation/xmlcoder, I noticed that some of the doc comments are missing, and we made some of the protocols
publicby mistake. I've added doc comments with example code and update those protocols accordingly.Additionally, tests and a section in
README.mdfor recently added property wrappers were missing, that's fixed as well.