Skip to content

Conversation

@ApurveKaranwal
Copy link
Contributor

Fixes

Fixes #380

Description

Replaced all unsafe os.system() calls with subprocess.run() for safer command execution and proper error handling.
Added a check so that github-readme-to-html is installed only if it’s missing.

Technical details

  • Uses subprocess.run() with check=True to catch errors
  • Checks for npx availability before running commands
  • Creates dist/ folder if it doesn’t exist
  • Provides clear status messages for installation and conversion

Tests

  1. Run python script.py -i README.md -o output.html -t "My Project"
  2. Confirm that:
    • The HTML file is generated in dist/
    • github-readme-to-html installs only if missing
    • Proper error messages appear if input file is missing

Checklist

  • My pull request has a descriptive title (not a vague title like Update index.md).
  • My pull request targets the default branch of the repository (main or master).
  • My commit messages follow best practices.
  • My code follows the established code style of the repository.
  • I added or updated tests for the changes I made (if applicable).
  • I added or updated documentation (if applicable).
  • I tried running the project locally and verified that there are no
    visible errors.

@netlify
Copy link

netlify bot commented Oct 17, 2025

Deploy Preview for fosscprogresstracker canceled.

Name Link
🔨 Latest commit 2c9d980
🔍 Latest deploy log https://app.netlify.com/projects/fosscprogresstracker/deploys/68f254942e911000089f89d0

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you 🙏🏻 for contributing to progress-tracker by @FOSS-Community. Looking Forward for more contributions.

script.py Outdated
# Check input file exists
# ------------------------------
if not Path(FILE_NAME).is_file():
print(f"Error: The input file does not exist: {FILE_NAME}", file=sys.stderr)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's better to raise an exception here, instead of sys.exit

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay sir, i will fix both sys.exit to raise an exception.

script.py Outdated
# Check if npx is installed
# ------------------------------
if not shutil.which("npx"):
print("Error: 'npx' is not installed. Please install Node.js first: https://nodejs.org/", file=sys.stderr)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here also, use exception instead of sys.exit

Copy link
Member

@Mr-Sunglasses Mr-Sunglasses left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🎊 , Thanks @ApurveKaranwal

@Mr-Sunglasses Mr-Sunglasses merged commit 8a4f9bd into FOSS-Community:main Oct 17, 2025
5 checks passed
@ApurveKaranwal
Copy link
Contributor Author

LGTM 🎊 , Thanks @ApurveKaranwal

Thank you so much, Sir! Contributing to FOSSC has been an amazing experience for me. I truly enjoyed working on this issue, learning along the way, and seeing my contribution get merged into the main branch. I’m really motivated to keep improving and contribute more in the future!🚀

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Replace os.system() with subprocess.run() and improve npm installation check

2 participants