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

build: introduce foundational base library #15968

Merged
merged 4 commits into from
Jan 5, 2024
Merged

Conversation

dotnwat
Copy link
Member

@dotnwat dotnwat commented Jan 5, 2024

The base library is a foundational library. The rough guidelines for determining
if something belongs in the base library are:

  1. It would be reasonable if every target depended on it (e.g. vlog.h).
  2. It contains no dependencies on other Redpanda targets (e.g. utils).
  3. Its external dependencies may also be considered foundational (e.g. seastar).

Currently this includes things like seastarx.h, vlog.h, etc...

This is a necessary step towards header isolation, because we can no longer default include the root directory.

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v23.3.x
  • v23.2.x
  • v23.1.x

Release Notes

  • none

Adds <module>/include path to includes directory when linking against a
v::lib library, which is intended to be the pattern for exposing
public headers from a module.

The default include path of the root of the tree is also removed because
it is currently handled in the root cmake file and will later be
removed.

Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
The base library is a foundational library. The rough guidelines for determining
if something belongs in the base library are:

1. It would be reasonable if every target depended on it (e.g. vlog.h).
2. It contains no dependencies on other Redpanda targets (e.g. utils).
3. Its external dependencies may also be considered foundational (e.g. seastar).

Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
rockwotj
rockwotj previously approved these changes Jan 5, 2024
Copy link
Contributor

@rockwotj rockwotj left a comment

Choose a reason for hiding this comment

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

❤️ it!

I hope the header moves were automated 😄

@@ -0,0 +1,6 @@
The base library is a foundational library. The rough guidelines for determining
Copy link
Contributor

Choose a reason for hiding this comment

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

I really like having READMEs in every module for documentation.

@@ -1 +1,7 @@
v_cc_library(NAME base)

rp_test(
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be in the standard tests directory?

Copy link
Member Author

Choose a reason for hiding this comment

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

ah, yeh, thanks. will push an extra commit after we see if this things builds in CI

@dotnwat
Copy link
Member Author

dotnwat commented Jan 5, 2024

❤️ it!

I hope the header moves were automated 😄

my second favorite webpage on the internet is https://blog.jasonmeridth.com/posts/use-git-grep-to-replace-strings-in-files-in-your-git-repository/

@dotnwat dotnwat marked this pull request as ready for review January 5, 2024 19:50
@dotnwat dotnwat requested review from BenPope and a team as code owners January 5, 2024 19:50
@dotnwat dotnwat requested review from savex, bharathv, ztlpn and graphcareful and removed request for a team January 5, 2024 19:50
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Copy link
Contributor

@graphcareful graphcareful left a comment

Choose a reason for hiding this comment

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

+1 sweet

@dotnwat dotnwat requested a review from rockwotj January 5, 2024 22:30
@vbotbuildovich
Copy link
Collaborator

@dotnwat dotnwat merged commit d09b7dc into redpanda-data:dev Jan 5, 2024
20 checks passed
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.

None yet

5 participants