add haggit's migration script to scripts directory#720
add haggit's migration script to scripts directory#720haggit-eliyahu wants to merge 2 commits intomainfrom
Conversation
|
❌ Marketplace Validation Failed Click to view the full reportValidation Report🧩 IntegrationsPre-Build Stagef5_big_iq
observe_it
websense
outpost24
fire_eye_ex
mc_afee_active_response
mc_afee_atd
lastline
site24x7
illusive_networks
|
Summary of ChangesHello, 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 introduces a new automation tool designed to streamline the migration and refactoring process for integrations. By leveraging LibCST for code manipulation and integrating with existing repository utilities, the script ensures consistency across migrated projects, handles complex dependency updates, and automates repetitive maintenance tasks like versioning and license management. 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. 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. 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 introduces scripts/refactor_integration.py, a utility for automating the refactoring of integrations, including widget conversion, code deconstruction, and test migration. The review feedback identifies several style guide violations, specifically recommending the use of the pipe operator for type unions and the consistent use of pathlib.Path methods for file system operations.
| class SDKInstanceTransformer(cst.CSTTransformer): | ||
| """Replaces strict isinstance checks on SDK objects with hasattr checks.""" | ||
|
|
||
| def leave_Call(self, original_node: cst.Call, updated_node: cst.Call) -> Union[cst.Call, cst.BaseExpression]: |
There was a problem hiding this comment.
According to the style guide, the pipe operator (|) should be used for unions instead of Union for Python 3.10+.
| def leave_Call(self, original_node: cst.Call, updated_node: cst.Call) -> Union[cst.Call, cst.BaseExpression]: | |
| def leave_Call(self, original_node: cst.Call, updated_node: cst.Call) -> cst.Call | cst.BaseExpression: |
References
- Use the pipe operator for unions (e.g., str | None) instead of Optional ( Python 3.10+). (link)
|
|
||
| def leave_ImportFrom( | ||
| self, original_node: cst.ImportFrom, updated_node: cst.ImportFrom | ||
| ) -> Union[cst.ImportFrom, cst.RemovalSentinel]: |
There was a problem hiding this comment.
According to the style guide, the pipe operator (|) should be used for unions instead of Union for Python 3.10+.
| ) -> Union[cst.ImportFrom, cst.RemovalSentinel]: | |
| ) -> cst.ImportFrom | cst.RemovalSentinel: |
References
- Use the pipe operator for unions (e.g., str | None) instead of Optional ( Python 3.10+). (link)
| """The core engine for refactoring integrations.""" | ||
|
|
||
| def __init__( | ||
| self, integrations_root: Path, dst_path: Path, tests_dir: Path, integrations_list: Optional[str] = None |
There was a problem hiding this comment.
According to the style guide, the pipe operator (|) should be used for unions instead of Optional for Python 3.10+.
| self, integrations_root: Path, dst_path: Path, tests_dir: Path, integrations_list: Optional[str] = None | |
| self, integrations_root: Path, dst_path: Path, tests_dir: Path, integrations_list: str | None = None |
References
- Use the pipe operator for unions (e.g., str | None) instead of Optional ( Python 3.10+). (link)
| for json_file in widgets_dir.glob("*.json"): | ||
| logger.debug(f"Processing widget file: {json_file.name}") | ||
| try: | ||
| with open(json_file, "r", encoding="utf-8") as f: |
There was a problem hiding this comment.
The style guide mandates the use of pathlib.Path for all file system operations. Please use Path.open() or Path.read_text() instead of the built-in open() function.
| with open(json_file, "r", encoding="utf-8") as f: | |
| with json_file.open("r", encoding="utf-8") as f: |
References
- Always use pathlib.Path for file system operations. (link)
| with open(json_file, "r", encoding="utf-8") as f: | ||
| data = json.load(f) | ||
| converted = self._transform_widget_data(data) | ||
| with open(json_file, "w", encoding="utf-8") as f: |
There was a problem hiding this comment.
The style guide mandates the use of pathlib.Path for all file system operations. Please use Path.open() or Path.write_text() instead of the built-in open() function.
| with open(json_file, "w", encoding="utf-8") as f: | |
| with json_file.open("w", encoding="utf-8") as f: |
References
- Always use pathlib.Path for file system operations. (link)
| for root, _, _ in os.walk(tests_dest_path): | ||
| (Path(root) / "__init__.py").touch(exist_ok=True) |
There was a problem hiding this comment.
The style guide mandates the use of pathlib.Path for all file system operations. os.walk should be replaced with a pathlib-based iteration.
| for root, _, _ in os.walk(tests_dest_path): | |
| (Path(root) / "__init__.py").touch(exist_ok=True) | |
| for path in [tests_dest_path, *tests_dest_path.rglob("*")]: | |
| if path.is_dir(): | |
| (path / "__init__.py").touch(exist_ok=True) |
References
- Always use pathlib.Path for file system operations. (link)
| def _add_local_deps(path: Path): | ||
| local_path = get_local_packages_path() | ||
|
|
||
| def find_latest_whl(package_name: str, subfolder: str) -> Optional[str]: |
There was a problem hiding this comment.
According to the style guide, the pipe operator (|) should be used for unions instead of Optional for Python 3.10+.
| def find_latest_whl(package_name: str, subfolder: str) -> Optional[str]: | |
| def find_latest_whl(package_name: str, subfolder: str) -> str | None: |
References
- Use the pipe operator for unions (e.g., str | None) instead of Optional ( Python 3.10+). (link)
| if not pyproject_path.is_file(): | ||
| return | ||
|
|
||
| with open(pyproject_path, "r", encoding="utf-8") as f: |
There was a problem hiding this comment.
The style guide mandates the use of pathlib.Path for all file system operations. Please use Path.open() instead of the built-in open() function.
| with open(pyproject_path, "r", encoding="utf-8") as f: | |
| with pyproject_path.open("r", encoding="utf-8") as f: |
References
- Always use pathlib.Path for file system operations. (link)
| logger.debug(f"Bumping version from {data['project']['version']} to {new_v}") | ||
| data["project"]["version"] = new_v | ||
|
|
||
| with open(pyproject_path, "w", encoding="utf-8") as f: |
There was a problem hiding this comment.
The style guide mandates the use of pathlib.Path for all file system operations. Please use Path.open() instead of the built-in open() function.
| with open(pyproject_path, "w", encoding="utf-8") as f: | |
| with pyproject_path.open("w", encoding="utf-8") as f: |
References
- Always use pathlib.Path for file system operations. (link)
| "item_name": name, | ||
| "publish_time": datetime.now().strftime("%Y-%m-%d"), | ||
| }) | ||
| with open(rn_path, "a", encoding="utf-8") as f: |
There was a problem hiding this comment.
The style guide mandates the use of pathlib.Path for all file system operations. Please use Path.open() instead of the built-in open() function.
| with open(rn_path, "a", encoding="utf-8") as f: | |
| with rn_path.open("a", encoding="utf-8") as f: |
References
- Always use pathlib.Path for file system operations. (link)
1 participant
No description provided.