[libsolutil] Add new JSON helper functions. - #13579
Conversation
|
I wish #11967 would be merged before getting these helpers in, as it may remove the need for them. |
It was working a bunch of times, but nobody reviewed it in time and rebasing causes a bunch of work. |
|
If you're sure it will be mergeable, we could commit to reviewing it this time. We're focusing on killing off stale PRs now so yours would be a prime target :) |
|
Need to rebase it again and finish off new failures, I can try looking at it on Monday. |
|
I did a brief look on #11967. I was trying to fix some tests there - from my point of view it still needs some work to fix all tests. I think it is much easier to just merge this PR. |
|
I'm fine with merging this if #11967 is not ready. We can always just remove these helpers in that other PR if it makes them obsolete. I just want to have these merged so that we can get to the important part, i.e. the asm import. |
|
Needs rebase. gp2 is fixed on |
|
Just pushed a commit to de-macro the helpers, since macros hurt my eye :-). There'd be other ways to do that, but this is probably good enough. |
ekpyron
left a comment
ekpyron
left a comment
There was a problem hiding this comment.
Fine with me, but since I added the de-macro commit, another review may be nice.
nikola-matic
left a comment
nikola-matic
left a comment
There was a problem hiding this comment.
Personally, I'm good with this, and templates are always nicer than macros. Please take a look at this comment thread just in case, and if you agree, make the necessary changes. Otherwise, merge away.
Only thing left is naming of get, which should be fine.
Of course it is easier to merge this, but also makes it harder to ever complete the other one 😓 |
5 participants
Add new helper functions to
libsolutil/JSON.hto make #12834 more readable.