-
Notifications
You must be signed in to change notification settings - Fork 925
Added contributor.md and updated code block styles to better fix github markdown patterns #1997
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?
Conversation
…ub markdown patterns
🎉 All Contributor License Agreements have been signed. Ready to merge. |
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.
Pull Request Overview
This PR adds a new CONTRIBUTOR.md
file and standardizes documentation by converting inline command snippets to fenced bash code blocks and unifying Python invocation.
- Adds detailed contributor guidelines in
CONTRIBUTOR.md
- Replaces inline shell commands with fenced
bash
blocks across docs - Updates instructions to use
python
instead ofpython3
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
tests/README.md | Converted commands to fenced bash blocks and updated Python call |
examples/docker/README.md | Wrapped Docker build command in fenced bash block |
examples/README.md | Standardized venv and install commands with fenced bash blocks |
README.md | Fenced installation commands in bash blocks |
INSTALL.md | Unified Python invocation and added fenced bash blocks |
DEVELOPER.md | Added setup section and fenced build/docs/unasync commands |
CONTRIBUTOR.md | Introduced new contributor guidelines documentation |
Comments suppressed due to low confidence (4)
tests/README.md:17
- Consider specifying "Python 3.x" explicitly instead of just "python", since on some systems
python
may point to Python 2 or be unavailable.
A python env suitable for running tests:
INSTALL.md:11
- On environments where
python
defaults to Python 2 or is not installed, consider usingpython3
or adding a note about requiring Python 3.
python -m pip install confluent-kafka
DEVELOPER.md:80
- The code fence here wraps both the
python tools/unasync.py
command and the explanatory comment. It may improve clarity to separate the two into distinct fenced blocks or move the comment outside the block.
```bash
CONTRIBUTOR.md:27
- Relative link to
../../issues
may not resolve correctly in GitHub Markdown; consider using an absolute path like/issues
or linking directly to the repository's issues page.
[issues](../../issues)
This comment has been minimized.
This comment has been minimized.
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.
First pass comments.
This comment has been minimized.
This comment has been minimized.
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.
Thanks for addressing previous comments. Just a couple of comments more.
## Development Environment Setup | ||
|
||
### Prerequisites | ||
|
||
- Python 3.7 or higher | ||
- Git | ||
- librdkafka (for Kafka functionality) | ||
|
||
### Setup Steps | ||
|
||
1. **Fork and Clone** | ||
```bash | ||
git clone https://github.com/your-username/confluent-kafka-python.git | ||
cd confluent-kafka-python | ||
``` | ||
|
||
2. **Create Virtual Environment** | ||
```bash | ||
python -m venv venv | ||
source venv/bin/activate # On Windows: venv\Scripts\activate | ||
``` | ||
|
||
3. **Install librdkafka** (if not already installed) | ||
- See the main README.md for platform-specific installation instructions | ||
|
||
4. **Install Dependencies** | ||
```bash | ||
pip install -e .[dev,tests,docs] | ||
``` | ||
|
||
5. **Verify Setup** | ||
```bash | ||
python -c "import confluent_kafka; print('Setup successful!')" | ||
``` | ||
|
||
## Build | ||
|
||
$ python -m build | ||
```bash | ||
python -m build | ||
``` | ||
|
||
If librdkafka is installed in a non-standard location provide the include and library directories with: | ||
|
||
$ C_INCLUDE_PATH=/path/to/include LIBRARY_PATH=/path/to/lib python -m build | ||
```bash | ||
C_INCLUDE_PATH=/path/to/include LIBRARY_PATH=/path/to/lib python -m build | ||
``` | ||
|
||
**Note**: On Windows the variables for Visual Studio are named INCLUDE and LIB | ||
|
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 have build section after install. Let's rearranging these sections.
|
||
Documentation will be generated in `docs/_build/`. | ||
|
||
or: | ||
|
||
$ python setup.py build_sphinx | ||
```bash | ||
python setup.py build_sphinx |
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.
Since we are touching these commands, we should either use python
or python3
in all of our docs. I think python3
is better as that is present in all the distros. Same with pip3
.
We can do this right now as we have already removed support for Python 2.7.
Adds contributor markdown and cleans up some of documentation. This is an effort as a precursor to engaging the community for completely.
This adds a Contributor.md file, changes the setup instructions to include virtual environment, removes the python2 vs 3 implication for older OS defaults, changes code references to code blocks, and adds some cross md file references where relevant.
Checklist
References
JIRA:
Test & Review
Open questions / Follow-ups
This had been discussed internally ahead of time, nothing surprising for confluent reviewers should pop up.
Initial changes were drafted with cursor then hand touched and reviewed to ensure intent and quality. Recommended that instructions are clear and no typos or mistakes are present in a read through.