hse-project / hse

HSE: Heterogeneous-memory storage engine
https://hse-project.github.io
673 stars 65 forks source link

clang-format prep work #592

Closed alexttx closed 1 year ago

alexttx commented 1 year ago

Includes changes we want (and in some cases need) before reformatting code with clang-format.

alexttx commented 1 year ago

This PR results in this layout (only testing files have changed in this PR):

Internal libs, linked with HSE library:

Directory                                  #include Pattern
---------                                  ----------------
lib/config/include/hse/config              <hse/config/...>
lib/error/include/hse/error                <hse/error/...>
lib/logging/include/hse/logging            <hse/logging/...>
lib/mpool/include/hse/mpool                <hse/mpool/...>
lib/pidfile/include/hse/pidfile            <hse/pidfile/...>
lib/rest/include/hse/rest                  <hse/rest/...>
lib/util/include/hse/util                  <hse/util/...>

One-off (doesn't quite fit the internal lib pattern):

Directory                                  #include Pattern
---------                                  ----------------
lib/include/hse/ikvdb                      <hse/ikvdb/...>

CLI lib (not linked w/ HSE library):

Directory                                  #include Pattern
---------                                  ----------------
cli/include/hse/cli                        <hse/cli/...>

Testing:

Directory                                  #include Pattern
---------                                  ----------------
tests/fixtures/include/hse/test/fixtures   <hse/test/fixtures/...>
tests/framework/include/hse/mtf            <hse/mtf/...>
tests/mocks/include/hse/mock               <hse/mock/...>
tests/mocks/repository/include/hse/mocks   <hse/mocks/...>
tests/support/include/hse/test/support     <hse/test/support/...>

Tools (for kmt, putbin, etc):

Directory                                  #include Pattern
---------                                  ----------------
tools/include/hse/tools                    <hse/tools/...>
alexttx commented 1 year ago

I think it might make more sense to continue using the hse/test include path, instead of hse/mtf/framework.h use hse/test/framework/mtf.h. And the hse/mocks/api.h instead of hse/mock/api.h. But other than that thanks for fixing this.

Clarify? Maybe suggest a new layout for testing in the format of my previous comment.

tristan957 commented 1 year ago

Essentially what I am saying is this:

- #include <hse/mtf/framework.h>
- #include <hse/mock/api.h>
+ #include <hse/test/framework/mtf.h>
+ #include <hse/mocks/api.h>

Saves you two include patterns since these just piggyback on ones that already exist.

tristan957 commented 1 year ago

Reiterating that I left a few comments in the other PR that I think should be addressed before this gets committed

alexttx commented 1 year ago

@tristan957

Reiterating that I left a few comments in the other PR that I think should be addressed before this gets committed

I think I addressed them all.

alexttx commented 1 year ago

What's the value in having format hooks in meson? To use that I have to have a build directory.

beckerg commented 1 year ago

I think it's a good idea to have a command to reformat one's tree so that I don't have to know how to run it myself. And if you want to run it by hand such scripts shouldn't preclude you from doing that, of course... Plus, I presume the check-in machinery will need a rule in order to reject non-comformant code.

Speaking of, what's the incantation to reformat one's tree via meson?

tristan957 commented 1 year ago

What's the value in having format hooks in meson? To use that I have to have a build directory.

Good point. Could be removed then.

tristan957 commented 1 year ago

I think it's a good idea to have a command to reformat one's tree so that I don't have to know how to run it myself. And if you want to run it by hand such scripts shouldn't preclude you from doing that, of course... Plus, I presume the check-in machinery will need a rule in order to reject non-comformant code.

Speaking of, what's the incantation to reformat one's tree via meson?

ninja -C build format currently, but seems like it could just be removed.

beckerg commented 1 year ago

I think we want that, otherwise you have to socialize how to do it otherwise, better to just encode the knowledge into a simple-to-use target. Not clear why it needs a build dir, why doesn't just operate on the src directly?

tristan957 commented 1 year ago

@beckerg for a ninja target to be generated, you need Meson to create the build directory.