Skip to content

Remove dead code handling missing dependencies#11627

Merged
cdrini merged 1 commit intointernetarchive:masterfrom
cdrini:refactor/rm-some-dead-code
Dec 25, 2025
Merged

Remove dead code handling missing dependencies#11627
cdrini merged 1 commit intointernetarchive:masterfrom
cdrini:refactor/rm-some-dead-code

Conversation

@cdrini
Copy link
Collaborator

@cdrini cdrini commented Dec 25, 2025

Small refactor ; these dependencies are always available. Also make the code more robust and ensure sanitize always runs.

Technical

Testing

Tested adding XSS descriptions to book pages does not cause XSS and is sanitized.

Screenshot

Stakeholders

@github-project-automation github-project-automation bot moved this to Waiting Review/Merge from Staff in Ray's Project Dec 25, 2025
@cdrini cdrini marked this pull request as ready for review December 25, 2025 19:00
Copilot AI review requested due to automatic review settings December 25, 2025 19:00
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes dead code that handled optional dependencies (genshi and BeautifulSoup) by converting conditional imports to direct imports, since these dependencies are always available in the project.

Key changes:

  • Removed try-except blocks around genshi and BeautifulSoup imports, making them direct imports
  • Removed the runtime check for genshi availability in the sanitize() function
  • Refactored error handling to use recursive sanitization instead of nested try-except blocks

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@cdrini cdrini force-pushed the refactor/rm-some-dead-code branch from e74747b to ccd5f3d Compare December 25, 2025 19:02
Copy link
Collaborator

@RayBB RayBB left a comment

Choose a reason for hiding this comment

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

The changes look right to me and if it works locally then I'd say we're good to go. But how do you test this?

@cdrini
Copy link
Collaborator Author

cdrini commented Dec 25, 2025

Sweet! Updated description with testing instructions

@cdrini cdrini merged commit fa93233 into internetarchive:master Dec 25, 2025
4 checks passed
@github-project-automation github-project-automation bot moved this from Waiting Review/Merge from Staff to Done in Ray's Project Dec 25, 2025
@cdrini cdrini deleted the refactor/rm-some-dead-code branch December 25, 2025 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants