Added Encode/Decode traits (and new_element helper) #48
Conversation
|
Yes it should work fine, however I don't see the point in this pr. I think adding everything together makes more sense. I can redo my work on error-handling, that's fine. |
|
I'd much rather split the work into smaller chunks because imo it's easier to work on bit by bit (in the time I have available), to share work, and to reasonably review each set of changes. I'm also afraid of ending up in the same situation as #37 where PRs stall and because of the size of the changes rebasing is basically impossible / completely manual. I could have a go at updating #37 but it basically involves manually reapplying any changes since then and the diffs are so huge it's nearly impossible to observe any regression. |
src/types.rs
Outdated
| type Object; | ||
| /// Parsing error | ||
| type Error; | ||
| /// Parse an XML/SVD element into an SVD Object. |
There was a problem hiding this comment.
Sentence doesn't feel right to me. Maybe
[...] into it's corresponding
Object
src/types.rs
Outdated
| pub trait Encode { | ||
| /// Encoding error | ||
| type Error; | ||
| /// Encode an SVD object into an XML element |
There was a problem hiding this comment.
Same here as above. Maybe just skip "an SVD object".
src/types.rs
Outdated
| pub trait Parse { | ||
| /// Object returned by parse method | ||
| type Object; | ||
| /// Parsing error | ||
| type Error; | ||
| /// Parse an XML/SVD element into an SVD Object. | ||
| /// Parse an XML/SVD element into it's corresponding Object. |
There was a problem hiding this comment.
Object refers to the trait type, put into code block. i.e `Object`
|
I still feel uneasy about merging this without anything related to it. Maybe it would actually be easier to open a refactor branch and just directly push to it and then merge it when finished, as you first suggested. We could have discussions over on irc or another suitable "forum" |
|
If you'd like I could update a couple of objects and open dependent PRs as
well so we can test / prove the interface? Then it's easy to merge em also.
…On Sat, 10 Mar 2018, 1:40 AM Emil Gardström, ***@***.***> wrote:
I still feel uneasy about merging this without anything related to it.
Maybe it would actually be easier to open a refactor branch and just
directly push to it and then merge it, as you first suggested.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#48 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AA0hzGvdyI3XMe4s8ykS8RzBqICjXQZlks5tcnhHgaJpZM4SgClB>
.
|
|
I say yes to implementing this on some objects, but no to opening dependent PRs. If we are going to make multiple small PRs depending on this I think skipping the PR system until it is complete is the best choice. |
|
Closed in favour of upstream/refactor |
Traits for encoding and decoding of SVD objects <-> XML Elements towards #46.
@Emilgardis I think this should be compatible with your approach to error handling?