search

apache / kvrocks

Apache Kvrocks is a distributed key value NoSQL database that uses RocksDB as storage engine and is compatible with Redis protocol.
https://kvrocks.apache.org/
Apache License 2.0
3.57k stars 468 forks source link

Improve the code style by introducing macros #2521

Open git-hulk opened 2 months ago

git-hulk commented 2 months ago

Followup code improvements mentioned in PR #2508. Currently, we check and return error if the status is not ok in many many places, which can be improved by introducing macros.

The naming candidates can be:

and for converting the rocksdb status if there is an error:

Originally posted by @mapleFU in https://github.com/apache/kvrocks/pull/2508#pullrequestreview-2276461263

git-hulk commented 2 months ago

@AntiTopQuark Would you like to take this issue?

AntiTopQuark commented 2 months ago

@AntiTopQuark Would you like to take this issue?

I’d be happy to take on this issue. ：）

PragmaTwice commented 2 months ago

The first macro is duplicated with the current GET_OR_RET macro.

PragmaTwice commented 2 months ago

For the second I think it depends on the context, e.g. rocksdb::Status -> Status? rocksdb::Status -> rocksdb::Status?

We can extend GET_OR_RET for the latter, and the former doesn't require new macro.

git-hulk commented 2 months ago

For the second I think it depends on the context, e.g. rocksdb::Status -> Status? rocksdb::Status -> rocksdb::Status?

We can extend GET_OR_RET for the latter, and the former doesn't require new macro.

Yes, my bad. GET_OR_RET allows ignoring the return value, so the former one is unnecessary.