LCORE-240: Makefile targets + updated documentation #222
Conversation
WalkthroughThe Makefile was updated to support Python package distribution and uploading, introducing new targets and a registry variable. The README.md was expanded to document these new publishing steps, including instructions for configuring the registry and authentication. Additional make targets were also documented in the usage section. Changes
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
Makefile (2)
4-6: Prefer?=to keep the variable truly overridableUsing a simple
=prevents a value coming from the environment (e.g.export PYTHON_REGISTRY=testpypi) from taking precedence.
Switching to?=lets the env / CLI override and still keeps the default.-PYTHON_REGISTRY = pypi +PYTHON_REGISTRY ?= pypi
71-78:verifytarget exceeds checkmake’s max-body lengthStatic analysis already flags this. Wrap the individual linters into a helper target to shorten the body:
+lint-all: + $(MAKE) black pylint pyright ruff docstyle check-types + verify: ## Run all linters - $(MAKE) black - $(MAKE) pylint - $(MAKE) pyright - $(MAKE) ruff - $(MAKE) docstyle - $(MAKE) check-types + $(MAKE) lint-allThis keeps the same behaviour and silences
maxbodylength.README.md (4)
26-29: Fix list indentation to satisfy Markdown lintersCurrent four-space indentation violates MD007. Two spaces are enough:
-* [Publish the service as Python package on PyPI](#publish-the-service-as-python-package-on-pypi) - * [Generate distribution archives to be uploaded into Python registry](#generate-distribution-archives-to-be-uploaded-into-python-registry) - * [Upload distribution archives into selected Python registry](#upload-distribution-archives-into-selected-python-registry) - * [Packages on PyPI and Test PyPI](#packages-on-pypi-and-test-pypi) +* [Publish the service as Python package on PyPI](#publish-the-service-as-python-package-on-pypi) + * [Generate distribution archives to be uploaded into Python registry](#generate-distribution-archives-to-be-uploaded-into-python-registry) + * [Upload distribution archives into selected Python registry](#upload-distribution-archives-into-selected-python-registry) + * [Packages on PyPI and Test PyPI](#packages-on-pypi-and-test-pypi)
247-257: Specify language for fenced blocksMD040 flags these blocks. Adding
bashhelps syntax highlighting & tooling:-``` +```bash make distribution-archives @@ -``` +```bash make upload-distribution-archives
265-273: Mark.pypircfence as INI for clarity-``` +```ini [testpypi] @@ pypi-{your-API-token}--- `281-282`: **Convert bare URLs to reference links** Avoid MD034 by wrapping them: ```diff -* https://pypi.org/project/lightspeed-stack/ -* https://test.pypi.org/project/lightspeed-stack/0.1.0/ +* [PyPI](https://pypi.org/project/lightspeed-stack/) +* [Test PyPI 0.1.0](https://test.pypi.org/project/lightspeed-stack/0.1.0/)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Makefile(2 hunks)README.md(3 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
27-27: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
28-28: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
29-29: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
247-247: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
255-255: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
265-265: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
281-281: Bare URL used
(MD034, no-bare-urls)
282-282: Bare URL used
(MD034, no-bare-urls)
🪛 LanguageTool
README.md
[style] ~251-~251: Consider using a different verb to strengthen your wording.
Context: ... make distribution-archives ``` Please make sure that the archive was really built to av...
(MAKE_SURE_ENSURE)
🪛 checkmake (0.2.2)
Makefile
[warning] 71-71: Target body for "verify" exceeds allowed length of 5 (6).
(maxbodylength)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-pr
| upload-distribution-archives: ## Upload distribution archives into Python registry | ||
| pdm run python -m twine upload --repository ${PYTHON_REGISTRY} dist/* | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fail fast when no artefacts exist
twine expands dist/*; when the glob is empty the shell passes the literal string and twine aborts. Guard against this:
upload-distribution-archives: ## Upload distribution archives into Python registry
- pdm run python -m twine upload --repository ${PYTHON_REGISTRY} dist/*
+ @if [ -z "$$(ls -1 dist 2>/dev/null)" ]; then \
+ echo "No distribution files found in ./dist – did you run make distribution-archives?"; exit 1; \
+ fi
+ pdm run python -m twine upload --repository ${PYTHON_REGISTRY} dist/*📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| upload-distribution-archives: ## Upload distribution archives into Python registry | |
| pdm run python -m twine upload --repository ${PYTHON_REGISTRY} dist/* | |
| upload-distribution-archives: ## Upload distribution archives into Python registry | |
| @if [ -z "$$(ls -1 dist 2>/dev/null)" ]; then \ | |
| echo "No distribution files found in ./dist – did you run make distribution-archives?"; exit 1; \ | |
| fi | |
| pdm run python -m twine upload --repository ${PYTHON_REGISTRY} dist/* |
🤖 Prompt for AI Agents
In the Makefile at lines 82 to 84, the upload-distribution-archives target runs
twine upload on dist/* without checking if any files exist, causing twine to
fail if the glob is empty. Modify the target to first check if any files exist
in the dist directory before running twine, and if none exist, exit with an
error or skip the upload to fail fast and avoid passing a literal glob pattern
to twine.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
Makefile (1)
83-85: Guard against emptydist/*before calling TwinePrevious review already pointed out that
twineaborts when the glob is empty; the safeguard is still missing.upload-distribution-archives: ## Upload distribution archives into Python registry - pdm run python -m twine upload --repository ${PYTHON_REGISTRY} dist/* + @if [ -z "$$(ls -1 dist 2>/dev/null)" ]; then \ + echo "No distribution files found in ./dist – did you run make distribution-archives?"; exit 1; \ + fi + pdm run python -m twine upload --repository ${PYTHON_REGISTRY} dist/*
🧹 Nitpick comments (3)
Makefile (2)
4-6: Use?=to allow effortless override ofPYTHON_REGISTRYA conditional assignment (
?=) lets users (or CI jobs) override the registry via environment/CLI without having to passmake PYTHON_REGISTRY=… --no-builtin-variables.-PYTHON_REGISTRY = pypi +PYTHON_REGISTRY ?= pypi
71-78:verifytarget exceeds recommended body length – split for clarityStatic analysis flags 6 commands (>5). Consider composing it from smaller phony targets (
lint-python,lint-docs, etc.) and invoke them here to keep the body terse and self-describing.README.md (1)
247-273: Add language identifiers to fenced code blocksMarkdown-lint (
MD040) warns about missing languages; adding one improves syntax highlighting and copy-paste UX.-``` -make distribution-archives -``` +```bash +make distribution-archives +``` … -``` -make upload-distribution-archives -``` +```bash +make upload-distribution-archives +``` … -``` -[testpypi] +```ini +[testpypi] username = __token__ password = pypi-{your-API-token} [pypi] username = __token__ password = pypi-{your-API-token} -``` +``` </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between e76c9d16306acb34a51fc520d989271c2124721d and e4805cca245c72c4b0fa023bad57818b121a3ee0. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `Makefile` (2 hunks) * `README.md` (3 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 checkmake (0.2.2)</summary> <details> <summary>Makefile</summary> [warning] 71-71: Target body for "verify" exceeds allowed length of 5 (6). (maxbodylength) </details> </details> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>README.md</summary> 27-27: Unordered list indentation Expected: 2; Actual: 4 (MD007, ul-indent) --- 28-28: Unordered list indentation Expected: 2; Actual: 4 (MD007, ul-indent) --- 29-29: Unordered list indentation Expected: 2; Actual: 4 (MD007, ul-indent) --- 247-247: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 255-255: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 265-265: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 281-281: Bare URL used (MD034, no-bare-urls) --- 282-282: Bare URL used (MD034, no-bare-urls) </details> </details> <details> <summary>🪛 LanguageTool</summary> <details> <summary>README.md</summary> [style] ~251-~251: Consider using a different verb to strengthen your wording. Context: ... make distribution-archives ``` Please make sure that the archive was really built to av... (MAKE_SURE_ENSURE) </details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)</summary> * GitHub Check: build-pr </details> <details> <summary>🔇 Additional comments (1)</summary><blockquote> <details> <summary>Makefile (1)</summary> `79-81`: **👍 `dist/` cleanup added – stale artefacts issue solved** The extra `rm -rf dist` prevents upload collisions. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
1 participant
Description
LCORE-240: Makefile targets + updated documentation
Type of change
Related Tickets & Documents
Summary by CodeRabbit
New Features
Documentation