Skip to content

Conversation

@ShaharNaveh
Copy link
Collaborator

@ShaharNaveh ShaharNaveh commented Nov 1, 2025

@youknowone would you object to move https://github.com/RustPython/__doc__.git to this project? (in it's own crate)

Summary by CodeRabbit

  • Chores
    • Removed internal documentation comments from code with no functional impact or changes to user-facing features.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 1, 2025

Walkthrough

Documentation comments were removed from six PyStr methods (removeprefix, removesuffix, format_map, title, isprintable, istitle) in the builtins module without modifying their implementations or behavior.

Changes

Cohort / File(s) Summary
Documentation Removal
vm/src/builtins/str.rs
Removed public doc comments from methods: removeprefix, removesuffix, format_map, title, isprintable, istitle

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • No functional logic changes; purely documentation removal
  • Consistent pattern applied across multiple method declarations
  • No risk of introducing bugs or altering behavior

Poem

A rabbit hops through comments old,
Where docs once spoke, now silence gold—
Six methods bare, their words now freed,
Simplicity is all we need! 🐰✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "Remove user defined docstrings for builtins.str" directly and accurately describes the primary change in the changeset. The raw summary confirms that the main modification is the removal of public documentation comments for several PyStr methods in vm/src/builtins/str.rs, with no functional code changes. The title is concise, specific, and uses clear language without vague terms or unnecessary noise, making it easily understandable for someone scanning the project history.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 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 e096ce7 and 390f9f9.

📒 Files selected for processing (1)
  • vm/src/builtins/str.rs (0 hunks)
💤 Files with no reviewable changes (1)
  • vm/src/builtins/str.rs
⏰ 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). (10)
  • GitHub Check: Check Rust code with rustfmt and clippy
  • GitHub Check: Run snippets and cpython tests on wasm-wasi
  • GitHub Check: Run snippets and cpython tests (windows-latest)
  • GitHub Check: Run snippets and cpython tests (ubuntu-latest)
  • GitHub Check: Check the WASM package and demo
  • GitHub Check: Run rust tests (macos-latest)
  • GitHub Check: Run snippets and cpython tests (macos-latest)
  • GitHub Check: Run rust tests (windows-latest)
  • GitHub Check: Run rust tests (ubuntu-latest)
  • GitHub Check: Ensure compilation on various targets

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 a7e8ac6 into RustPython:main Nov 1, 2025
12 checks passed
@youknowone
Copy link
Member

@ShaharNaveh I forgot what was the major reason to split it out. Probably the license and workflow part. To integrate it again, guidelines:

  • Please check the generate script is still working well.
  • The license must be aligned to CPython
  • We'd better to generate them and commit from CI. (manual trigger when upgrading version)

@ShaharNaveh
Copy link
Collaborator Author

@ShaharNaveh I forgot what was the major reason to split it out. Probably the license and workflow part. To integrate it again, guidelines:

  • Please check the generate script is still working well.

I intend to refractor it, either way I'll make it will work.

  • The license must be aligned to CPYTHON

Sure, will copy CPython license to crates/rustpython_doc_db/LICENSE

  • We'd better to generate them and commit from CI. (manual trigger when upgrading version)

I thought of having the generate script to have a --check flag that will exit with non-zero on any diff, we will check it on the CI.

@youknowone
Copy link
Member

I thought of having the generate script to have a --check flag that will exit with non-zero on any diff, we will check it on the CI.

The current script is depends on python runtime. Each platform generate their own set of document. To cover the entire platform, the results from different platforms must be merged.

@ShaharNaveh
Copy link
Collaborator Author

I thought of having the generate script to have a --check flag that will exit with non-zero on any diff, we will check it on the CI.

The current script is depends on python runtime. Each platform generate their own set of document. To cover the entire platform, the results from different platforms must be merged.

Make sense, I don't expect nt to be available on Linux. But I don't think that there's a difference between docstring of same object on different platforms, right? i.e, str.count is the same on Windows, Linux, MacOS. correct?

@youknowone
Copy link
Member

Yes, they are.

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