Precommit fix #4
Conversation
Summary of ChangesHello @binary-husky, 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 enhancing the project's code quality and maintainability. It refines the pre-commit setup by replacing multiple code style enforcement tools with a more focused utility for removing unused imports. Concurrently, it systematically cleans up redundant import statements and resolves minor formatting inconsistencies across a broad spectrum of files, resulting in a leaner and more consistent codebase. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe 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 by creating a comment using either
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 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request applies pre-commit fixes, primarily removing unused imports and correcting formatting across many files. While these cleanups are beneficial, this PR introduces two major regressions. First, the pre-commit configuration in .pre-commit-config.yaml has been significantly weakened by replacing black, isort, and flake8 with only autoflake. This removes crucial code formatting and linting capabilities, which can lead to inconsistent code style and missed bugs. I strongly recommend restoring a robust linting and formatting setup. Second, the test file tests/test_networking.py has been deleted, removing test coverage for the networking utilities in ajet/utils/networking.py. Removing tests is highly discouraged and reduces confidence in the codebase's correctness. These tests should be restored.
| - repo: https://github.com/myint/autoflake | ||
| rev: v2.2.0 | ||
| hooks: | ||
| - id: flake8 | ||
| additional_dependencies: [flake8-docstrings] | ||
| args: [ | ||
| "--max-line-length=100", | ||
| "--max-complexity=20", | ||
| "--select=C,E,F,W,B,B950", | ||
| "--ignore=E203,E266,E501,W503", | ||
| ] | ||
| - id: autoflake | ||
| args: [ | ||
| --in-place, | ||
| --remove-all-unused-imports, | ||
| --ignore-init-module-imports | ||
| ] |
There was a problem hiding this comment.
Replacing black, isort, and flake8 with only autoflake is a significant step back for code quality. autoflake only handles unused imports, while the previous tools enforced code style and caught a wide range of potential bugs.
I strongly recommend using a more comprehensive tool. ruff is a modern, high-performance tool that can replace flake8, isort, autoflake, and even black's formatting. Here is a suggested configuration using ruff for both linting and formatting.
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.2.1
hooks:
- id: ruff
args: [--fix, --exit-non-zero-on-fix]
- id: ruff-format| except Exception: | ||
|
|
||
| return "127.0.0.1" | ||
| return "127.0.0.1" |
There was a problem hiding this comment.
The test file for this utility, tests/test_networking.py, was removed in this pull request. Removing tests is highly discouraged as it reduces confidence in the code's correctness and can lead to regressions. Please restore the test file.
| from typing import List, Tuple, Union | ||
| from typing import List, Union, Tuple, Dict, Optional, Any | ||
| from typing import List, Union, Tuple, Dict, Optional |
There was a problem hiding this comment.
These two import statements from typing are partially redundant. They can be merged into a single, sorted import statement for better readability and maintainability.
| from typing import List, Tuple, Union | |
| from typing import List, Union, Tuple, Dict, Optional, Any | |
| from typing import List, Union, Tuple, Dict, Optional | |
| from typing import Dict, List, Optional, Tuple, Union |
No description provided.