msggen: fix walk through nested json schemas#7218
msggen: fix walk through nested json schemas#7218cdecker merged 1 commit intoElementsProject:masterElementsProject/lightning:masterfrom daywalker90:msggen-fix-walk-nesteddaywalker90/lightning:msggen-fix-walk-nestedCopy head branch name to clipboard
Conversation
cdecker
left a comment
There was a problem hiding this comment.
Looks good, but I'd like the if statement to be clarified a bit, as the precedence of the conditions is not clear, and left-to-right order is even a tautology.
7c6a32e to
d440033
Compare
|
need #7217 merged first I had to merge some arrays so it got more complicated :/ |
1227d9a to
4da2e18
Compare
|
I WILL BRING ORDER TO THIS AAAAHHH I'm honestly not sure why i lost the deterministic ordering of the fields of all the generated code but now its |
4da2e18 to
0be4b26
Compare
|
|
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
0be4b26 to
e54288a
Compare
|
We derived the i32 from rust enums implicitly by their order in the enum. I added explicit numbers in #7217 so the sorting doesn't mess anything up. Btw if we ever delete an "open" enum field in *.proto with |
We might want to keep dummy placeholders in there once we do. But we'll cross that bridge once we get to it. I don't fully understand why sorting is required here, does it have to do with the iteration through nested fields? Otherwise it feels strange to have enums not be in numeric order, but no real issue, I'm just curious :-) |
|
Yes when merging the arrays that were previously not collected we would get enums in node.proto that were out of order. And protoc wants them to start at 0. The ordering of enums in the rust files could be removed eagain after the explicit enum values were added. I don't remember exactly why i sorted those to be honest. Want me to check that out? |
e54288a to
fa6c8f2
Compare
|
rebased and rust model enums sorted by number not name |
Changelog-None
fa6c8f2 to
c93d955
Compare
|
Rebased on top of |
|
i have a bunch more coming but i wait until this and #7230 is merged |
I finally found (what i suspected in the first place) the bug in
msggenwhere it wasn't walking through theif's correctly (you may have noticed the weird "x doesn't have a type" logs.Fixes: #7185 #5345 #5354
Replaces: #7210