Skip to content
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

optimised runcmd func in tools/icu/icutrim.py #48241

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jaisipani
Copy link

Replaced the usage of os.system with subprocess.check_call for better security and error handling.

Used a try-except block to handle CalledProcessError exceptions raised by subprocess.check_call.

Return the return code of the command execution (e.returncode) when an error occurs, to allow better handling in the calling code.

Replaced the usage of os.system with subprocess.check_call for better security and error handling.

Used a try-except block to handle CalledProcessError exceptions raised by subprocess.check_call.

Return the return code of the command execution (e.returncode) when an error occurs, to allow better handling in the calling code.
@nodejs-github-bot nodejs-github-bot added i18n-api Issues and PRs related to the i18n implementation. icu Issues and PRs related to the ICU dependency. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. labels May 29, 2023
@avivkeller
Copy link
Member

Thanks for the PR! When you get a chance, please lint your commit message to abide by the guidelines we have in place. Other than that, looks good :-).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
i18n-api Issues and PRs related to the i18n implementation. icu Issues and PRs related to the ICU dependency. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants