Add back SCM - #12
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughBumps VERSION 3.4.0 → 3.4.1 and updates Pythonista Bluetooth stubs: removes a type-ignore on an internal import, adds a SharedCentralManager subclass, a Changes
Sequence Diagram(s)sequenceDiagram
actor App
participant cb as cb module
participant SCM as SharedCentralManager
participant Del as _CentralManagerDelegate
App->>cb: set_central_delegate(delegate)
cb->>SCM: shared_manager.delegate = delegate
App->>cb: set_verbose(true)
cb->>SCM: shared_manager.verbose = true
App->>cb: scan_for_peripherals()
cb->>SCM: shared_manager.start_scan()
SCM-->>Del: delegate callbacks
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @hbmartin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request primarily focuses on updating the project's version and refining the type stubs for Pythonista's Core Bluetooth (cb) module. It increments the version number and reintroduces or clarifies the SharedCentralManager class and its associated global instance within the cb.pyi stub file. This change likely aims to improve type checking and provide more accurate autocompletion for users interacting with the Core Bluetooth functionalities in Pythonista, specifically regarding the shared central manager.
Highlights
- Version Update: The project version has been incremented from
3.4.0to3.4.1. - Type Hint Refinement: The
type: ignore[import-not-found]comment has been removed from the_cbimport incb.pyi, suggesting improved module discoverability or type checking accuracy. - Shared Central Manager Reintroduction: The
SharedCentralManagerclass, along with itsdelegate,verboseproperties, andverbose_logmethod, has been added to thecb.pyistub file. A globalshared_managerinstance has also been introduced.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
Reviewer's guide (collapsed on small PRs)Reviewer's GuideBump version to 3.4.1 and update Bluetooth stubs by cleaning import directives, adding a SharedCentralManager subclass, and declaring a shared_manager global variable. Class diagram for SharedCentralManager and shared_manager additionsclassDiagram
class CentralManager
class _CentralManagerDelegate
class SharedCentralManager {
delegate: _CentralManagerDelegate | None
verbose: bool
verbose_log(): None
}
SharedCentralManager --|> CentralManager
shared_manager: SharedCentralManager | None
Flow diagram for PR changes overviewflowchart LR
A[VERSION file] -- "bump" --> B[3.4.1]
C[cb.pyi stubs] -- "add" --> D[SharedCentralManager class]
C -- "add" --> E[shared_manager variable]
C -- "clean" --> F[import statement]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly re-introduces SharedCentralManager and the shared_manager global to the cb.pyi stubs, enhancing type support for the library. The version bump and import cleanup are also appropriate. My review includes one suggestion to update the module's __all__ declaration to include the new symbols, which is important for maintaining a clear public API.
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to ffa9397 in 1 minute and 46 seconds. Click for details.
- Reviewed
37lines of code in2files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. VERSION:1
- Draft comment:
Version bump to 3.4.1 looks correct; consider adding a trailing newline for style consistency. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_Y8CQaZ0PXpJTjRLj
You can customize 
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
stubs/pythonista_stubs/cb.pyi (2)
30-60: Export new public symbols via all.You introduced
SharedCentralManagerandshared_manageras public, but they are not in__all__. In stubs,__all__constrains star-imported names; omitting these can hide them fromfrom cb import *and tooling that reads__all__.Apply this diff to add the new symbols to
__all__:__all__ = ( @@ "reset", "scan_for_peripherals", "set_central_delegate", "set_verbose", "stop_scan", + "SharedCentralManager", + "shared_manager", )
84-90: Initialize shared_manager to None for clarity and consistency.Declaring a default signals intended runtime value and aligns with how you annotated
delegateandverbose.Apply this diff:
-shared_manager: SharedCentralManager | None +shared_manager: SharedCentralManager | None = None
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
VERSION(1 hunks)stubs/pythonista_stubs/cb.pyi(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
stubs/pythonista_stubs/cb.pyi (1)
stubs/_cb.pyi (1)
CentralManager(83-111)
🔇 Additional comments (2)
VERSION (1)
1-1: Patch version bump looks correct for stub additions.3.4.1 is appropriate for adding non-breaking typings. No further action needed.
stubs/pythonista_stubs/cb.pyi (1)
7-28: _cb.pyi stub is included in the package
stubs/_cb.pyiis present in the repo.pyproject.tomlmapsstubs/to thepythonista_stubspackage andtool.setuptools.package-data"*": ["**/*"]ensures every.pyiunderstubs/(including_cb.pyi) is bundled.- There are no manifest entries that would omit
_cb.pyifrom the sdist or wheel.The
# type: ignore[import-not-found]can safely be dropped—_cb.pyiwill be discoverable by consumers’ type checkers.
|
@o-murphy how does this look? I defer to your expertise here so feel free to push other cb changes onto this branch |
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed d64a8c6 in 1 minute and 35 seconds. Click for details.
- Reviewed
34lines of code in1files - Skipped
0files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. stubs/pythonista_stubs/cb.pyi:88
- Draft comment:
Consider initializing 'delegate' to None if that's the intended default. Explicit defaults in stubs can clarify runtime behavior. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This is a type stub file where the primary purpose is to define types. The current annotation already indicates delegate can be None through the Union type. Adding an explicit default doesn't provide additional type information. The verbose field has a default likely because it represents a meaningful default state, while delegate being None may just be an uninitialized state. The comment could be valid if there's runtime behavior where delegate always starts as None, and making that explicit in stubs helps developers. The original code may have had a reason for omitting the default. While explicit defaults can be helpful, in a stub file the Union type already fully captures the type information. The runtime behavior is sufficiently documented by the type annotation alone. The comment should be deleted as it suggests a style preference that doesn't add meaningful type information in this context.
2. stubs/pythonista_stubs/cb.pyi:92
- Draft comment:
The global 'shared_manager' now explicitly defaults to None. Verify that this matches the intended initialization behavior at runtime. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to verify the intended behavior, which is against the rules. It does not provide a specific suggestion or point out a clear issue with the code.
3. stubs/pythonista_stubs/cb.pyi:51
- Draft comment:
New symbols ('SharedCentralManager' and 'shared_manager') are added to all. Consider grouping related symbols together for clarity and easier maintenance. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
4. stubs/pythonista_stubs/cb.pyi:7
- Draft comment:
Removal of the type ignore comment from the _cb import improves type safety. Confirm that _cb now provides the necessary type information. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_JTLfvbg963iGZKW7
You can customize
2 participants
PR Type
Enhancement
Description
Version bump from 3.4.0 to 3.4.1
Add
SharedCentralManagerclass to Bluetooth stubsRemove type ignore comment for
_cbimportAdd
shared_managerglobal variableDiagram Walkthrough
File Walkthrough
VERSION
Version bump to 3.4.1VERSION
cb.pyi
Add SharedCentralManager class and clean importsstubs/pythonista_stubs/cb.pyi
SharedCentralManagerclass extendingCentralManagershared_managerglobal variable declaration_cbimportSummary by Sourcery
Add SharedCentralManager stub and shared_manager global to bluetooth stubs, clean up import, and bump package version
New Features:
Enhancements:
Summary by CodeRabbit
Important
Add
SharedCentralManagerclass andshared_managervariable tocb.pyi, remove type ignore comment, and bump version to 3.4.1.SharedCentralManagerclass incb.pyi, extendingCentralManagerwithdelegateandverboseattributes.shared_managerglobal variable of typeSharedCentralManagerincb.pyi._cbimport incb.pyi.VERSION.This description was created by
for d64a8c6. You can customize this summary. It will automatically update as commits are pushed.