Skip to content

Conversation

@yangdanny97
Copy link
Contributor

@yangdanny97 yangdanny97 commented Nov 18, 2024

Summary

This is the inverse of UseTypesFromTyping https://github.com/Instagram/Fixit/blob/main/src/fixit/rules/use_types_from_typing.py

In Python versions after 3.10, we can replace typing.List and other aliases with the builtin equivalents.

Test Plan

make

closes #482

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 18, 2024
@yangdanny97
Copy link
Contributor Author

Oh, I just realized this is almost a duplicate of #482

This one adds typing.Type support which the other one doesn't have, but I'll let the maintainers decide which to move forward with.

@2803rishita
Copy link

🤖 Automated Code Review

📊 Pull Request Review Summary

Overview

This pull request proposes a change to replace type annotations from the typing module with their built-in equivalents, as Python versions after 3.10 support these built-ins directly. The changes primarily consist of additions to documentation and a new linting rule to enforce this best practice.

🎯 Key Findings

  • Total Issues Found: 0
  • Critical: 0
  • High: 0
  • Medium: 0
  • Low: 0

🔍 Detailed Issues

🔴 Critical Issues

None found.

🟠 High Priority Issues

None found.

🟡 Medium Priority Issues

None found.

🟢 Low Priority Issues

None found.


✅ Positive Aspects

  1. Alignment with Python Updates: The PR effectively aligns the codebase with Python 3.10 and future best practices by utilizing built-in types, which promotes code readability and maintainability.
  2. Informative Documentation: The documentation added in docs/guide/builtins.rst clearly explains the usage of the new linting rule, including valid and invalid examples, making it easy for developers to understand the changes.
  3. Comprehensive Implementation: The implementation in src/fixit/rules/use_builtin_types.py is well-structured and effectively utilizes classes from the libcst library for linting, demonstrating good familiarity with tools related to code analysis.

📝 Overall Assessment

This pull request is well-executed and enhances the clarity and maintainability of the codebase while promoting adherence to the latest Python standards. There are no issues found, and this PR can be merged without reservations. Great job to the author on these changes!

✅ No issues found! Code looks good.


Generated by PR Review Agent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants