Skip to content

Conversation

@ShaharNaveh
Copy link
Collaborator

@ShaharNaveh ShaharNaveh commented Nov 14, 2025

ref: #6214

Summary by CodeRabbit

  • Chores
    • Removed a workspace member from the project configuration.
    • Updated workspace dependency references to use workspace-managed packages instead of local paths.
    • Switched dev-dependency resolution to be handled by the workspace, simplifying dependency management and build configuration.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 14, 2025

Walkthrough

Removed the top-level jit workspace member and updated workspace dependency paths; adjusted the JIT crate's dev-dependency to use workspace = true instead of an explicit local path/version.

Changes

Cohort / File(s) Summary
Workspace manifest
Cargo.toml
Removed jit from workspace.members; updated [workspace.dependencies] to reference rustpython-jit at crates/jit instead of jit.
JIT crate manifest
crates/jit/Cargo.toml
Changed dev-dependency rustpython-derive from { path = "../derive", version = "0.4.0" } to workspace = true so it resolves via the workspace.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Focus points:
    • Confirm no remaining references to the removed top-level jit member in CI/build scripts.
    • Verify workspace dependency resolution produces a single expected version for rustpython-jit and rustpython-derive.
    • Ensure local development workflows (e.g., cargo commands, IDE workspace) still work with the new paths.

Suggested reviewers

  • youknowone

Poem

🐇 I nudged a crate from root to stack,
Paths adjusted — no looking back.
Derive now reads from workspace tree,
Quiet changes, tidy and free. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Move jit -> crates/jit' clearly and accurately summarizes the main change: relocating the jit component to a new directory structure under crates/.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 16db6f4 and 9170adc.

📒 Files selected for processing (1)
  • Cargo.toml (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-28T22:22:55.921Z
Learnt from: reactive-firewall
Repo: RustPython/RustPython PR: 6176
File: .github/workflows/Check_Tests.yml:133-141
Timestamp: 2025-09-28T22:22:55.921Z
Learning: In the RustPython project's CI-5974-Test-RustPython-Integration action, the override-rustpython-path input is marked as required but has runtime fallback logic that defaults to RUSTPYTHONPATH environment variable or "Lib" if neither is provided, making explicit specification unnecessary in most cases.

Applied to files:

  • Cargo.toml
🔇 Additional comments (1)
Cargo.toml (1)

151-151: Path update verified. All references correctly point to crates/jit with no orphaned paths.

The verification confirms that the move to crates/jit is complete and consistent:

  • Root Cargo.toml line 151 correctly updates the path to crates/jit
  • No dangling references to the old jit path exist
  • The moved crate is properly discoverable at the new location
  • The workspace glob pattern "crates/*" correctly includes the relocated crate

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@youknowone youknowone merged commit cc2e84b into RustPython:main Nov 15, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants