-
Notifications
You must be signed in to change notification settings - Fork 28
chore: remove shell flag usage in project scripts #538
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: gruebel <anton.gruebel@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @gruebel, 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 refactors the project's utility scripts to eliminate the use of shell=True
in subprocess.run
calls and replaces direct shell commands with their Python equivalents. This change significantly improves the security posture of the scripts by preventing potential shell injection vulnerabilities and enhances their robustness and cross-platform compatibility.
Highlights
- Security and Best Practices: All instances of
subprocess.run
that previously usedshell=True
have been refactored to pass commands as a list of arguments, enhancing security by mitigating shell injection risks. - Pythonic File Operations: Direct shell commands for file copying (
cp
) and removal (rm
) within thee2e
function have been replaced with native Pythonshutil.copytree
andpathlib.Path.unlink
operations, improving script maintainability and portability. - Code Clean-up: The
ruff: noqa: S602
directive, which suppressed warnings related toshell=True
, has been removed as it is no longer necessary.
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 by creating a comment using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands on the current page.
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 pull request 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. ↩
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #538 +/- ##
=======================================
Coverage 97.89% 97.89%
=======================================
Files 39 39
Lines 1849 1849
=======================================
Hits 1810 1810
Misses 39 39
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request effectively removes the use of shell=True
in subprocess.run
calls, which is a great security improvement. It also replaces shell commands like cp
and rm
with Python's standard library functions, making the scripts more robust and platform-independent. I have one suggestion regarding the use of shutil.copytree
to ensure it perfectly matches the behavior of the original cp
command and avoids potential side effects.
Signed-off-by: gruebel <anton.gruebel@gmail.com>
This PR
cp
andrm
commands with proper Python code