Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR introduces Just as a build orchestration system for the Oxen project. A new top-level Justfile coordinates multi-project operations by delegating to Rust and Python sub-project Justfiles. Each sub-project receives its own Justfile with standardized recipes (build, lint, check, test, doc). Project documentation is updated to highlight Just-based workflows as alternatives to direct tool invocations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
oxen-python/Develop.md (1)
61-69:⚠️ Potential issue | 🟡 MinorStale documentation section: still references Sphinx, but the project now uses pdoc.
The Justfile's
docrecipe usespdocandpyproject.tomllistspdocas a dev dependency. This section still instructs users to install and use Sphinx. Consider updating it to reflect the newjust doc/pdocworkflow.Suggested update
## Documentation -``` -brew install sphinx-doc -``` - -``` -cd oxen/docs -make install -``` +Generate Python API docs with pdoc: + +```bash +just doc # from oxen-python/ +# or directly: +uv run pdoc python/oxen -o docs +```
🧹 Nitpick comments (5)
README.md (1)
180-180: Minor inconsistency: thejustalternative here saysjust python buildbut Develop.md (line 23) saysjust build.The root README suggests
just python build(which runs from repo root), whileoxen-python/Develop.mdsaysjust build(which runs from the oxen-python directory). Both are technically correct depending on working directory, but it could confuse contributors. Consider adding a brief note about the context (repo root vs. sub-project directory).oxen-python/pyproject.toml (1)
15-27: Pre-existing: dev/build tools in runtimedependencies.
pytest,pytest-datadir,ruff, andmaturinare listed as runtime dependencies. End-users installingoxenaishouldn't need these. Consider moving them into the[dependency-groups] devgroup alongsidemypyandpdocin a future cleanup.oxen-python/Justfile (1)
4-6: Consider usinguv run maturin developfor consistency.
uv syncinstallsmaturininto the managed venv, but line 6 callsmaturin developdirectly which requires the venv to be activated ormaturinto be onPATH. All other tool invocations in this file useuv run— usinguv run maturin develophere would be consistent and work regardless of venv activation state.Proposed fix
build: uv sync --no-install-project - maturin develop + uv run maturin developoxen-rust/Justfile (1)
28-30: Aligntest-onlywithtestby usingcargo nextest runinstead of plaincargo test.The
testrecipe uses nextest for better parallelism and output, buttest-onlyfalls back to plaincargo test. For substring matching, use the simpler syntax:Proposed fix
# Run a specific test by name substring test-only NAME: - cargo test {{NAME}} + cargo nextest run {{NAME}}This uses nextest's built-in substring filter, which is more consistent with the main
testrecipe.oxen-rust/README.md (1)
305-317: Clarify the working directory for the manual CLI test command.Line 316 uses
cd cli-test && ...which is relative. Since this README lives atoxen-rust/README.md, it's implicitly relative tooxen-rust/, but a reader who runs from the repo root would end up in the wrong directory. Consider making this explicit:-cd cli-test && uv sync && uv run pytest tests -v +cd oxen-rust/cli-test && uv sync && uv run pytest tests -vOr add a short note like "from the
oxen-rust/directory".
malcolmgreaves
force-pushed
the
mg/justfile
branch
from
1af3e00 to
0acc83b
Compare
Adds a `Justfile` for development commands for the entire project. Adds specialized `oxen-rust` and `oxen-python` `Justfile`s too. Commands are scoped to the project: root-level runs for both. The following development commands are supported in both: - `just check`: type checks code - `just lint`: formats code & runs linters - `just build`: downloads all dependencies & builds all artifacts - `just test`: runs all tests - `just doc`: generates all documentation Both also have `just test-only TESTNAME` to run a single test. `oxen-rust` also has: - `just serve`: uses `bacon` to run `oxen-server` - `just test-cli`: runs the Python-based CLI integration tests And `mypy` was added to `oxen-python`'s development dependencies to add support for checking type annotations.
malcolmgreaves
force-pushed
the
mg/justfile
branch
2 times, most recently
from
b560d9d to
048b513
Compare
1 participant
Adds a
Justfilefor development commands for the entire project.Adds specialized
oxen-rustandoxen-pythonJustfiles too.Commands are scoped to the project: root-level runs for both.
The READMEs have been updated with
justinstall instructionsand explanations on how to use the various new
justcommands.justThe following development commands are supported in both:
just check: type checks codejust lint: formats code & runs lintersjust build: downloads all dependencies & builds all artifactsjust test: runs all testsjust doc: generates all documentationBoth also have
just test-only TESTNAMEto run a single test.oxen-rustalso has:just serve: usesbaconto runoxen-serverjust test-cli: runs the Python-based CLI integration testsmypyPython type checkingAnd
mypywas added tooxen-python's development dependencies toadd support for checking type annotations.