search

mit-dci / opencbdc-tx

A transaction processor for a hypothetical, general-purpose, central bank digital currency
Other
896 stars 198 forks source link

Arch linux snappy #175

Closed pr4u4t closed 1 year ago

pr4u4t commented 2 years ago

171 according to my last comment check_library_exists(snappy snappy_compress "" HAVE_SNAPPY) should be changed to find_library(SNAPPY_LIBRARY snappy REQUIRED) as this library is needed by LevelDB and if shared only exists on the system it must be added to target linking.

HalosGhost commented 2 years ago

solid Concept ACK. Can you rebase on trunk (to get to a single commit)? Then I'll test locally (looks like it'll be quick to test and merge).

HalosGhost commented 1 year ago

Can this be appropriately rebased and cherry-pick dc7dcc52e5c812827138ce5b3bc0d81b626b4589? I just got a chance to test and this PR including that commit corrects the clean-build issue. I'd love to get that merged ASAP (possibly today if you have time to cherry-pick and rebase).

HalosGhost commented 1 year ago

@pr4u4t I'd really like to get this merged ASAP. I'm happy to help you get it appropriately rebased and squashed, or I can cherry-pick your commits to a new branch, and merge it separately if that would be easier. Given that trunk is actually broken for anyone without an appropriate env already setup, I'll probably move forward with the later option in the next few days if I don't hear back.

pr4u4t commented 1 year ago

I may find time today or tomorrow to make a rebase I was busy with a thing that you mentioned in #176. I'm not that mad to shell client-cli. Command results for restd needs to be convertible to json or xml, for that I needed to create utility class to store command result take a look here. I also see a possibility to use TypedData for storing configuration #173. Configuration file may be designed to be more structured like the one from nginx or lighttpd and parsed using flex, bison couple. It's possible because configuration options are not deAlthough this interface is not yet complete as I need to make map and vector shared to get rid of copying.

HalosGhost commented 1 year ago

@pr4u4t I actually went ahead and setup a branch with the cherry-picking, rebasing/squashing done in case I didn't hear back. If you want to take a look and reconfirm your sign-off, I'd happily open a new PR from that branch and get it merged ASAP: https://github.com/mit-dci/opencbdc-tx/commit/afc129d406d3a53cbda5b9fd362cb7acc6eb881e

Note that there's really only one substantive change from what you submitted (squashing/commit-message/whitespace aside): the cmake find_library call didn't work for me correctly unless I referred to snappy rather than snappy_compress.

pr4u4t commented 1 year ago

I don't mind unless this changes gets commited. What is really interesting that both snappy and snappy_compressoptions work for me. While we can consider that as resolved, let's focus on changes that I've proposed for command handling as there is still more work to do.

HalosGhost commented 1 year ago

I've opened the new PR and will merge it as soon as it has >0 tested ACKs. Once it's merged, it will make sense for you to rebase your other PR(s) on top of it so they all have a shared base.

As for which things it makes sense to merge after that, I'll take another review ASAP.