-
Notifications
You must be signed in to change notification settings - Fork 166
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
Increase png size limit #250
Conversation
Deploying registry-devnet with Cloudflare Pages
|
Deploying registry-initiation-1 with Cloudflare Pages
|
Deploying registry-testnet with Cloudflare Pages
|
WalkthroughThe pull request modifies size limit constants for SVG and PNG files in the image optimization process. The Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
_api/src/optimizeImages.ts (2)
10-11
: Update the comments to reflect new size limits.The comments still show the old values (5KB, 50KB) which could be confusing. Update them to match the new limits.
-const SVG_SIZE_LIMIT = 10 * 1024 // 5KB -const PNG_SIZE_LIMIT = 100 * 1024 // 50KB +const SVG_SIZE_LIMIT = 10 * 1024 // 10KB +const PNG_SIZE_LIMIT = 100 * 1024 // 100KB
10-11
: Document the rationale for increasing size limits.While the changes align with the PR objective, it would be helpful to document:
- Why both SVG and PNG limits were increased
- What types of images required these higher limits
- Whether any alternatives (like better optimization) were considered
Consider adding these details in a comment block above the constants.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
_api/src/optimizeImages.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages: registry-devnet
🔇 Additional comments (1)
_api/src/optimizeImages.ts (1)
10-11
: Verify the impact of increased size limits.Doubling the size limits for both SVG and PNG files could impact storage and performance. Please ensure that:
- The increased PNG limit (100KB) aligns with your storage capacity planning
- The larger SVG files (up to 10KB) don't affect page load times
- These new limits have been tested with real-world image assets
Let's analyze the current image sizes in the repository:
✅ Verification successful
Size limit increases are safe based on current usage patterns
Analysis of existing images shows all PNG files are under 10KB and SVG files under 1KB, well below both the old and new limits. No files were found that would be affected by the increased limits.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find and analyze sizes of existing PNG and SVG files echo "PNG files exceeding previous limit (50KB) but under new limit (100KB):" find . -name "*.png" -type f -size +50k -size -100k -exec ls -lh {} \; echo -e "\nSVG files exceeding previous limit (5KB) but under new limit (10KB):" find . -name "*.svg" -type f -size +5k -size -10k -exec ls -lh {} \; echo -e "\nLargest PNG files:" find . -name "*.png" -type f -exec ls -lh {} \; | sort -rh | head -n 5 echo -e "\nLargest SVG files:" find . -name "*.svg" -type f -exec ls -lh {} \; | sort -rh | head -n 5Length of output: 1548
Summary by CodeRabbit