feat: Support pyre and basedpyright config #6
Conversation
Reviewer's Guide by SourceryThis pull request adds support for Pyre configuration files by introducing a new style file ( Class diagram for Pyre configurationclassDiagram
class PyreConfiguration {
+site_package_search_strategy: str
+source_directories: list
+strict: bool
+python_version: str
}
note for PyreConfiguration "Represents the structure of the .pyre_configuration file"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughThe changes update type-checking configuration files. In the mypy configuration file ( Changes
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @phuongfi91, 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!
Summary of Changes
This pull request introduces support for Pyre, a static type checker, within the project. It adds a new configuration file specifically for Pyre (nitpick_styles/generic-pyre.toml) and modifies the existing mypy configuration (nitpick_styles/generic-mypy.toml) by removing an incomplete feature flag. The Pyre configuration file defines settings for the type checker, including the search strategy for site packages, source directories, strict mode, and Python version. It also includes a basic .watchmanconfig file.
Highlights
- Pyre Support: Adds initial configuration and file presence checks for Pyre, a static type checker.
- Configuration Files: Introduces
nitpick_styles/generic-pyre.tomlwith settings for Pyre, and updatesnitpick_styles/generic-mypy.toml. - .watchmanconfig: Adds a basic
.watchmanconfigfile required for pyre.
Changelog
- nitpick_styles/generic-mypy.toml
- Removed the
enable_incomplete_feature = "NewGenericSyntax"line.
- Removed the
- nitpick_styles/generic-pyre.toml
- Added a new file to define the pyre configuration.
- Added metadata for pyre, including name and URL.
- Configured file presence checks for
.pyre_configurationand.watchmanconfig. - Configured file tags for
.pyre_configurationand.watchmanconfig. - Added JSON configuration for
.pyre_configuration, includingsite_package_search_strategy,source_directories,strict, andpython_version. - Added a basic
.watchmanconfigfile with empty JSON content.
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 is currently in preview and 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 to provide feedback.
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.
Did you know?
Static type checkers like Pyre and mypy can help catch errors early in the development process, potentially saving time and resources compared to finding them during runtime.
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. ↩
Code Review Agent Run #369a81Actionable Suggestions - 0Review Details
|
There was a problem hiding this comment.
Code Review
This pull request introduces support for Pyre, a static type checker, by adding a configuration file for Nitpick. This is a valuable addition to the project, as it allows for more comprehensive static analysis of Python code. The implementation is straightforward and well-structured.
Merge Readiness
The code changes are well-structured and introduce a valuable feature. I am unable to directly approve the pull request, and users should have others review and approve this code before merging. I recommend merging this pull request.
|
Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here. PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
There was a problem hiding this comment.
PR Overview
This PR adds configuration support for Pyre by introducing a new nitpick style configuration file and updates the MyPy configuration by removing the enable_incomplete_feature option.
- Add nitpick_styles/generic-pyre.toml with Pyre configuration details.
- Update nitpick_styles/generic-mypy.toml by removing an unused feature flag.
Reviewed Changes
| File | Description |
|---|---|
| nitpick_styles/generic-pyre.toml | New configuration file for Pyre support |
| nitpick_styles/generic-mypy.toml | Removal of enable_incomplete_feature for cleaner config |
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
|
Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here. 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.
Hey @phuongfi91 - I've reviewed your changes - here's some feedback:
Overall Comments:
- It might be helpful to include a brief explanation of what pyre is and why it's being added.
- Consider adding a validation step to ensure the
.pyre_configurationfile is valid JSON.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Changelist by BitoThis pull request implements the following key changes.
|
![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 (1)
nitpick_styles/generic-pyre.toml (1)
15-29: Pyre Configuration Block: Comprehensive Settings
The configuration block for ".pyre_configuration" is detailed, covering all required settings such assite_package_search_strategy,source_directories,strict, andpython_version. A minor note: verify that the use of triple-quoted multi-line strings does not introduce any unintended whitespace in the final JSON content, although this approach improves readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
nitpick_styles/generic-mypy.toml(0 hunks)nitpick_styles/generic-pyre.toml(1 hunks)
💤 Files with no reviewable changes (1)
- nitpick_styles/generic-mypy.toml
🔇 Additional comments (4)
nitpick_styles/generic-pyre.toml (4)
1-4: Metadata Block: Checks Out
The metadata section is well-defined, clearly specifying the project name "pyre" and providing a valid URL reference to the Pyre project.
5-8: Files Present Section: Clear Instructions
This section precisely indicates that the files ".pyre_configuration" and ".watchmanconfig" need to be created with specific content. The instructions are concise and easy to follow.
9-12: Files Tags Section: Appropriate Tagging
The tag definitions correctly assign the expected types: ".pyre_configuration" is tagged as both text and JSON, while ".watchmanconfig" is tagged as text. This helps in validating file content formats.
31-33: Watchman Config Block: Minimalistic and Correct
The configuration for ".watchmanconfig" effectively sets the file content to an empty JSON object, which satisfies the minimal requirements for Pyre.
phuongfi91
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
nitpick_styles/generic-basedpyright.toml (3)
5-7: Review Files Presence Configuration.
The configuration properly indicates that the file “pyrightconfig.json” must be present with a guiding message. Ensure that the message “Create the file with the contents below” meets your project’s standards for clarity.
25-47: Review Global Settings and Environment Configuration.
This section defines global settings such asdefineConstant,stubPath,pythonVersion,pythonPlatform, andextraPaths. While the values are specified using triple-quoted strings, consider if using native TOML types (e.g., bare booleans, standard strings) would be more appropriate unless the tool specifically requires this format.
49-507: Review Comprehensive Diagnostics Settings Configuration.
This file provides an extensive and well-documented set of diagnostic rules for the Pyright type checker. The inline comments (with links to documentation) help clarify the purpose and default severity levels for each setting. One point to consider: most values (e.g., boolean flags, diagnostic severity strings) are represented as triple-quoted strings. Confirm that this formatting is required by Nitpick; if not, adopting native TOML value types (e.g.,trueinstead of""" true""") could simplify maintenance and reduce potential parsing issues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
nitpick_styles/generic-basedpyright.toml(1 hunks)
🔇 Additional comments (2)
nitpick_styles/generic-basedpyright.toml (2)
1-3: Review Meta Information Section.
The meta section clearly defines the project name and URL. Please double-check that the URL ("https://github.com/DetachHead/basedpyright") is current and correct.
10-23: Review Pyright JSON Configuration Block.
The block starting with the documentation URLs and the settings for the JSON configuration (include, exclude, ignore) is clear and well-commented. Verify that the multiline string literal values correctly capture the intended file path elements and patterns for your project’s structure.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
nitpick_styles/generic-pyproject.toml (1)
113-117: New Linter Dependency & Minor Comment Typo CorrectionThe addition of
"pylint>=3.3.4"to the dev dependency group aligns with the PR’s objective of enhancing static analysis and linting capabilities. Please verify that this version is correct and compatible with your overall toolchain.Additionally, there is a minor typo in the inline comment on line 113: consider changing “fomatter” to “formatter” for clarity.
Tags:
Proposed Diff for the Comment Typo:
- # linter & fomatter + # linter & formatter
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
nitpick_styles/generic-flake8.toml (1)
68-77: Update to Ignore List: E501 Added to Delegate Line Length Enforcement
The addition of the E501 ignore rule is appropriate given that line-length checks are now expected to be handled by Ruff as noted in the comment. This change cleanly delegates responsibility away from flake8, preventing potential conflicts.
Code Review Agent Run #2cd992Actionable Suggestions - 1
Review Details
|
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
| "information" | ||
| """ |
There was a problem hiding this comment.
The first line of the file appears to be a stray "information" string that doesn't belong to any configuration setting. This might cause parsing issues with the TOML file. Consider removing this line if it's not part of a valid configuration.
Code suggestion
Check the AI-generated fix before applying
| "information" | |
| """ |
Code Review Run #2cd992
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
nitpick_styles/generic-trunk.toml (1)
44-71: Enhanced Linter Configuration: Updated Versions and Additional Linters
The linting section now updates several tools (e.g., upgradingbanditto version1.8.3) and includes new linters like"checkov@3.2.382","codespell@2.4.1","pyright", and"basedpyright". These improvements should boost static analysis coverage. Ensure that your CI/CD pipelines and local environments support these updated and new tools.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
nitpick_styles/generic-trunk.toml(1 hunks)
🔇 Additional comments (4)
nitpick_styles/generic-trunk.toml (4)
25-26: Plugin Source Update: New Reference and URI
The updated plugin source now usesref = "main-ng"and points tohttps://github.com/NextGenContributions/plugins, which matches the new repository structure.
32-33: Extended Runtime Support: Added Go and Node.js
The addition of"go@>=1.21.0"and"node@>=18.20.5"in the runtimes list broadens the supported environments. Ensure these versions meet your project’s minimum requirements.
35-37: Runtime Definitions: Python System Version
The new runtimes definitions block settingtype = "python"andsystem_version = "allowed"provides a flexible configuration for Python. This configuration appears correct—please verify that this aligns with your deployment expectations.
73-84: Customized Mypy Command: Handling Incremental Issues
The lint definitions now include a custom mypy command that adds the--no-incrementalflag, addressing known internal error issues. The accompanying comments and references provide useful context. Please verify that the${target}placeholder is correctly substituted during execution.
Code Review Agent Run #288b34Actionable Suggestions - 1
Review Details
|
| ref = "main-ng" | ||
| uri = "https://github.com/NextGenContributions/plugins" |
There was a problem hiding this comment.
The PR changes the plugin source from the official Trunk repository (trunk-io/plugins at version v1.6.7) to a fork (NextGenContributions/plugins at branch main-ng). This might introduce compatibility or security issues if the fork is not properly maintained or synchronized with the official repository. Consider whether this change is intentional and necessary.
Code suggestion
Check the AI-generated fix before applying
| ref = "main-ng" | |
| uri = "https://github.com/NextGenContributions/plugins" | |
| ref = "v1.6.7" | |
| uri = "https://github.com/trunk-io/plugins" |
Code Review Run #288b34
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
nitpick_styles/generic-trunk.toml (1)
87-88: Duplicate Lint Ignore Configuration
The lint ignore block specifieslinters = ["ALL"]on two consecutive lines. If this duplication is not intended, consider removing the redundant entry to avoid confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
nitpick_styles/generic-trunk.toml(1 hunks)nitpick_styles/generic-trunk.toml(2 hunks)
🔇 Additional comments (5)
nitpick_styles/generic-trunk.toml (5)
25-26: Plugin Source Update: Verify Fork Usage
The plugin reference has been updated from the official repository to the fork atNextGenContributions/pluginswithref = "main-ng". Please ensure this change is intentional and that the fork is well-maintained and secure compared to the original source.
32-33: New Runtime Support Added
The addition of"go@>=1.21.0"and"node@>=18.20.5"expands the runtime support. Confirm that all downstream tools and build processes are configured to handle these runtimes.
35-37: Runtime Definitions Block
The new definitions section specifyingtype = "python"andsystem_version = "allowed"is clear. Verify that this meets your version management strategy and integrates well with the rest of your runtime configurations.
44-72: Linter Configuration Updates
Multiple linter versions have been updated (e.g.,bandit@1.8.3) and additional linters such ascheckov@3.2.382,codespell@2.4.1,gitleaks@8.24.0,hadolint@2.12.1-beta,markdown-link-check@3.13.6,markdown-table-prettify@3.6.0,markdownlint-cli2@0.17.2, as well as new entries likepyrightandbasedpyrighthave been added. Please ensure that these changes are aligned with your CI pipeline and that any required extra configuration for these tools is provided elsewhere in the project.
73-83: Mypy Lint Definition Override
The custom mypy command now includes the--no-incrementalflag to work around known issues. The provided comments and references clearly explain the reasoning behind the override. This configuration appears appropriate and well-documented.
Code Review Agent Run #6534a8Actionable Suggestions - 0Review Details
|
Summary by Sourcery
New Features:
Summary by CodeRabbit
Summary by Bito
This PR updates and refactors trunk configuration by adding support for Pyre and basedpyright with extensive type checking settings. It removes outdated configuration lines including obsolete '.trunk/configs/' prefixes, updates plugin references, dependency versions, and entries for markdownlint, shellcheck, and yamllint. It also introduces Vale configuration while enhancing static analysis capabilities and streamlining configuration management.Unit tests added: False
Estimated effort to review (1-5, lower is better): 2