-
Notifications
You must be signed in to change notification settings - Fork 542
move/consolidate installation requirements #2178
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
base: master
Are you sure you want to change the base?
move/consolidate installation requirements #2178
Conversation
Signed-off-by: Alexa Kreizinger <alexakreizinger@gmail.com>
WalkthroughBuild prerequisites were removed from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: Alexa Kreizinger <alexakreizinger@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
installation/downloads/source/build-and-install.md (2)
9-16: Clarify platform-specificity of package names.The package list uses Ubuntu/Debian package naming conventions (e.g.,
build-essentials,libfl-dev,libssl-dev). Consider adding a note indicating that package names differ across Linux distributions (RedHat/CentOS usegcc,gcc-c++,flex-devel,openssl-devel, etc.; Alpine uses different naming) to help users on other platforms adapt the instructions.Consider adding a brief platform note similar to:
To build and install Fluent Bit from source, you must also install the following packages: +> **Note:** The package names below are for Ubuntu/Debian. Package names differ on other Linux distributions (e.g., RedHat/CentOS, Alpine). Refer to your distribution's package manager for equivalent packages. + - `bison` - `build-essentials`
18-18: Expand guidance on discovering plugin-specific dependencies.The note mentions that certain input/output plugins require additional components (Kafka is given as an example), but doesn't explain how users can discover which specific plugins need what. Consider linking to or mentioning where plugin documentation specifies their dependencies to improve discoverability.
You could enhance this with:
-Additionally, certain [input](../data-pipeline/inputs.md) or [output](../data-pipeline/outputs.md) plugins might depend on additional components. For example, some plugins require Kafka. +Additionally, certain [input](../data-pipeline/inputs.md) or [output](../data-pipeline/outputs.md) plugins might depend on additional components. For example, the Kafka input and output plugins require Kafka libraries. Check the documentation for your chosen plugins to identify any additional dependencies.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
installation/downloads.md(0 hunks)installation/downloads/source/build-and-install.md(1 hunks)
💤 Files with no reviewable changes (1)
- installation/downloads.md
cosmo0920
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found some of the glitches in the updated contents. Could you take a look?
| - Bison 3 or greater | ||
| - YAML headers | ||
| - OpenSSL headers | ||
| To build and install Fluent Bit from source, you must also install the following packages: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to clarify the minimum requirement of CMake version.
Currently, we use CMake 3.31.6 in our CI.
Plus, we're still supporting to compile on CentOS7.
We also use CMake 3.31.6 for that platform in CI.
So, the minimum requirements of CMake version could be become CMake 3.31.6 now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can add that :) similarly, do you think we need to add minimum versions for any of these other libraries?
|
|
||
| - `bison` | ||
| - `build-essentials` | ||
| - `cmake` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - `cmake` | |
| - `cmake` (version 3.31.6 or greater) |
Signed-off-by: Alexa Kreizinger <alexakreizinger@gmail.com>
this PR moves the general installation requirements into the doc for building from source, since it turns out that info is only applicable to building from source.
...however, there was already a requirements section in that doc, with slightly different info. so I combined those + added some info per one of our Chronosphere developers who shared what packages he needed to install on a fresh Ubuntu installation.
Summary by CodeRabbit