Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the Canonical Project Structure for Broker #390

Merged
merged 3 commits into from
Mar 14, 2024

Conversation

Neverlord
Copy link
Member

For the Canonical Project Structure, see
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1204r0.html.

With the new structure, we drop the include and src sub-directories and place the source files next to the header files under libbroker. Further, we place the unit tests next to the source file with .test.cc suffix.

Standalone tools have been placed alongside libbroker to the project root, whereas system/integration tests as well as benchmarks remain under tests, but are organized more consistently.

@Neverlord Neverlord force-pushed the topic/neverlord/canonical-project-structure branch 2 times, most recently from 3e3213b to 66bf0df Compare February 23, 2024 16:39
For the Canonical Project Structure, see
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1204r0.html.

With the new structure, we drop the `include` and `src` sub-directories
and place the source files next to the header files under `libbroker`.
Further, we place the unit tests next to the source file with `.test.cc`
suffix.

Standalone tools have been placed alongside `libbroker` to the project
root, whereas system/integration tests as well as benchmarks remain
under `tests`, but are organized more consistently.
@Neverlord Neverlord force-pushed the topic/neverlord/canonical-project-structure branch from 66bf0df to 3c64a31 Compare February 23, 2024 16:54
@Neverlord Neverlord marked this pull request as ready for review February 23, 2024 17:13
@Neverlord
Copy link
Member Author

@ckreibich most files have just been renamed. One notable exception is for the unit tests: I have dropped the manual #define SUITE ... from all of the tests and instead auto-generate the names via CMake. The test.hh header also has been renamed, so I had to change the include for this header in several files.

There are no functional changes. However, I did find obviously obsolete stuff like tests/cpp/ssl.cc (tests APIs that have been removed a long while ago) and test/micro-benchmark/src/streaming.cc (also uses long removed APIs). I've dropped these as part of the cleanup here since there's no point in trying to restore either one as the APIs under test simply aren't there anymore.

There are several tests and benchmarks that we currently don't build (commented out in CMake). I'll check each of those individually as a followup to see what can be restored and what might be obsolete (or became superfluous, e.g., due to a new btest).

@Neverlord Neverlord requested a review from ckreibich February 23, 2024 17:14
@Neverlord
Copy link
Member Author

@ckreibich any ETA on this? I'd really like to get this in before doing tackling other issues (like #391) to avoid conflicts.

@@ -1,6 +1,6 @@
find_package(benchmark QUIET)

# add_subdirectory(cluster)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the rest of these benchmarks coming back later? I know there's an open issue for the cluster one.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I plan to make a pass over them later. Some of them seem outdated, though. So probably not all of them will come back.

@Neverlord Neverlord merged commit 4f11dba into master Mar 14, 2024
22 checks passed
@Neverlord Neverlord deleted the topic/neverlord/canonical-project-structure branch March 14, 2024 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants