Ed Hennis

Results 183 comments of Ed Hennis

@treeol This looks ready to merge, but part of our process requires the original author to sign off that it's ready after all the approvals are done. Could you just...

> I like this. If possible, consider whether the `TransStatus` and `TxnSql` enumerations can be combined. I _think_ that `TransStatus` is only "internal" to the running binary, so it should...

@yinyiqian1 Are we happy with the commit message being just ``` Add AMMClawback Transaction (XLS-0073d) (#5142) ``` ? If not, please suggest the details you'd like to include.

I just renumbered to use XLS-45d. I'm going to leave the branch name the same, though.

> So we do not have a PR enabling more features at once ? 1. I don't think there's any harm in enabling more than one feature at once. 2....

> > So we do not have a PR enabling more features at once ? > > 1. I don't think there's any harm in enabling more than one feature...

Would it be fair to say this PR is "trivial"?

Not a review yet, but this is such a cool idea.

Bad news right out the gate on Windows builds: ``` $ cmake build/cmake/msvc19.ON -- Using Conan toolchain: C:/Dev/rippled/review1/build/conan.msvc19/build/generators/conan_toolchain.cmake -- Conan toolchain: C++ Standard 20 with extensions OFF -- Conan toolchain:...

It's building successfully now. Neat workaround. Super annoying that this is an issue, though. I remember there were some `file(CREATE_LINK` calls elsewhere that were simply disabled under Windows. Would it...