Add SVG icon validation to pull requests - #24
Conversation
Document and enforce fill-only 16x16 SVG source rules. Validate path data and run lint tests in pull request CI. Normalize existing icons to satisfy the enforced constraints.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThe repository adds a standalone SVG linter, Vitest coverage, lint and test scripts, build-time linting, updated contribution guidance, and a pull-request workflow using Node.js LTS and pnpm. ChangesSVG validation workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant GitHubActions
participant pnpm
participant lintIcons
participant SVGO
PullRequest->>GitHubActions: open or update pull request
GitHubActions->>pnpm: install with frozen lockfile
GitHubActions->>pnpm: run test
GitHubActions->>pnpm: run build
pnpm->>lintIcons: run SVG linting
lintIcons-->>pnpm: return validation status
pnpm->>SVGO: optimize SVGs
SVGO-->>GitHubActions: return build status
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Deploying icons with
|
| Latest commit: |
354a29e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://18956a26.icons-3vy.pages.dev |
| Branch Preview URL: | https://ci-icon-validation.icons-3vy.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.nvmrc (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin the Node.js version used by the repository.
Both GitHub Actions workflows read
.nvmrc, solts/*can select a different Node.js major after a future LTS transition. Set.nvmrcto the exact version used by CI and local development, and updateAGENTS.mdwhen the version changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.nvmrc at line 1, Replace the moving lts/* value in .nvmrc with the exact Node.js version used by CI and local development, then update the Node.js version reference in AGENTS.md to match.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pull-request.yml:
- Line 14: Update the actions/checkout step in the pull-request workflow to set
persist-credentials to false before running pnpm install, pnpm test, and pnpm
build, while preserving the existing checkout behavior.
In `@AGENTS.md`:
- Around line 70-73: Update both documented command checklists in AGENTS.md to
include pnpm test alongside pnpm install, pnpm lint, and pnpm run build.
In `@lintIcons.js`:
- Around line 33-42: Update the attribute validation loop in lintIcons.js to
reject every on* SVG attribute, including onclick, using per-element allowlists
for explicitly permitted event attributes. Add a regression test covering a
disallowed executable attribute and verify the corresponding element is
rejected.
---
Nitpick comments:
In @.nvmrc:
- Line 1: Replace the moving lts/* value in .nvmrc with the exact Node.js
version used by CI and local development, then update the Node.js version
reference in AGENTS.md to match.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 817bd3f3-525a-4000-9f0e-186df7547b11
⛔ Files ignored due to path filters (129)
icons/AddFilled.svgis excluded by!**/*.svgicons/Adjust.svgis excluded by!**/*.svgicons/ArrowDown.svgis excluded by!**/*.svgicons/ArrowDownFilled.svgis excluded by!**/*.svgicons/ArrowLeft.svgis excluded by!**/*.svgicons/ArrowRight.svgis excluded by!**/*.svgicons/ArrowUp.svgis excluded by!**/*.svgicons/ArrowUpDown.svgis excluded by!**/*.svgicons/ArrowUpRight.svgis excluded by!**/*.svgicons/ArrowsLeftRight.svgis excluded by!**/*.svgicons/Ascending.svgis excluded by!**/*.svgicons/Back.svgis excluded by!**/*.svgicons/Bank.svgis excluded by!**/*.svgicons/BarChartInitia.svgis excluded by!**/*.svgicons/Bridge.svgis excluded by!**/*.svgicons/Burn.svgis excluded by!**/*.svgicons/Buy.svgis excluded by!**/*.svgicons/Calendar.svgis excluded by!**/*.svgicons/CancelCircle.svgis excluded by!**/*.svgicons/CaretDown.svgis excluded by!**/*.svgicons/CaretLeft.svgis excluded by!**/*.svgicons/CaretRight.svgis excluded by!**/*.svgicons/CaretUp.svgis excluded by!**/*.svgicons/Chains.svgis excluded by!**/*.svgicons/Chart.svgis excluded by!**/*.svgicons/Check.svgis excluded by!**/*.svgicons/CheckCircle.svgis excluded by!**/*.svgicons/CheckCircleFilled.svgis excluded by!**/*.svgicons/ChevronDown.svgis excluded by!**/*.svgicons/ChevronLeft.svgis excluded by!**/*.svgicons/ChevronRight.svgis excluded by!**/*.svgicons/ChevronUp.svgis excluded by!**/*.svgicons/ChevronsDown.svgis excluded by!**/*.svgicons/ChevronsRight.svgis excluded by!**/*.svgicons/CircleDashed.svgis excluded by!**/*.svgicons/Clock.svgis excluded by!**/*.svgicons/ClockFilled.svgis excluded by!**/*.svgicons/Close.svgis excluded by!**/*.svgicons/CloseCircle.svgis excluded by!**/*.svgicons/CloseCircleFilled.svgis excluded by!**/*.svgicons/Contact.svgis excluded by!**/*.svgicons/ContactBook.svgis excluded by!**/*.svgicons/Copy.svgis excluded by!**/*.svgicons/Diamond.svgis excluded by!**/*.svgicons/Dice.svgis excluded by!**/*.svgicons/Discord.svgis excluded by!**/*.svgicons/DocsFilled.svgis excluded by!**/*.svgicons/DotsVertical.svgis excluded by!**/*.svgicons/Download.svgis excluded by!**/*.svgicons/DragHandle.svgis excluded by!**/*.svgicons/DropInitia.svgis excluded by!**/*.svgicons/Ecosystem.svgis excluded by!**/*.svgicons/Edit.svgis excluded by!**/*.svgicons/Export.svgis excluded by!**/*.svgicons/ExternalLink.svgis excluded by!**/*.svgicons/Filter.svgis excluded by!**/*.svgicons/FilterSearch.svgis excluded by!**/*.svgicons/Forum.svgis excluded by!**/*.svgicons/Gauge.svgis excluded by!**/*.svgicons/Gavel.svgis excluded by!**/*.svgicons/Github.svgis excluded by!**/*.svgicons/GppMaybeFilled.svgis excluded by!**/*.svgicons/Grid.svgis excluded by!**/*.svgicons/GridThin.svgis excluded by!**/*.svgicons/Heart.svgis excluded by!**/*.svgicons/HeartFilled.svgis excluded by!**/*.svgicons/Hide.svgis excluded by!**/*.svgicons/History.svgis excluded by!**/*.svgicons/Home.svgis excluded by!**/*.svgicons/Hourglass.svgis excluded by!**/*.svgicons/IUSD.svgis excluded by!**/*.svgicons/Image.svgis excluded by!**/*.svgicons/Import.svgis excluded by!**/*.svgicons/Info.svgis excluded by!**/*.svgicons/InfoFilled.svgis excluded by!**/*.svgicons/LayoutSidebar.svgis excluded by!**/*.svgicons/Link.svgis excluded by!**/*.svgicons/List.svgis excluded by!**/*.svgicons/ListDetails.svgis excluded by!**/*.svgicons/ListThin.svgis excluded by!**/*.svgicons/Lock.svgis excluded by!**/*.svgicons/Maximize.svgis excluded by!**/*.svgicons/Member.svgis excluded by!**/*.svgicons/Menu.svgis excluded by!**/*.svgicons/Minus.svgis excluded by!**/*.svgicons/More.svgis excluded by!**/*.svgicons/MoreVert.svgis excluded by!**/*.svgicons/Note.svgis excluded by!**/*.svgicons/Notification.svgis excluded by!**/*.svgicons/Pay.svgis excluded by!**/*.svgicons/Plus.svgis excluded by!**/*.svgicons/Pool.svgis excluded by!**/*.svgicons/PoolSparkles.svgis excluded by!**/*.svgicons/Privacy.svgis excluded by!**/*.svgicons/QrCode.svgis excluded by!**/*.svgicons/QuestionFilled.svgis excluded by!**/*.svgicons/Refresh.svgis excluded by!**/*.svgicons/ReplayFilled.svgis excluded by!**/*.svgicons/Reward.svgis excluded by!**/*.svgicons/Sad.svgis excluded by!**/*.svgicons/Search.svgis excluded by!**/*.svgicons/Selector.svgis excluded by!**/*.svgicons/Send.svgis excluded by!**/*.svgicons/Setting.svgis excluded by!**/*.svgicons/SettingFilled.svgis excluded by!**/*.svgicons/ShoppingBag.svgis excluded by!**/*.svgicons/Show.svgis excluded by!**/*.svgicons/Shuffle.svgis excluded by!**/*.svgicons/SignOut.svgis excluded by!**/*.svgicons/Star.svgis excluded by!**/*.svgicons/StarFilled.svgis excluded by!**/*.svgicons/Swap.svgis excluded by!**/*.svgicons/SwapVert.svgis excluded by!**/*.svgicons/Tag.svgis excluded by!**/*.svgicons/Trash.svgis excluded by!**/*.svgicons/Twitter.svgis excluded by!**/*.svgicons/Unlock.svgis excluded by!**/*.svgicons/User.svgis excluded by!**/*.svgicons/UserSquare.svgis excluded by!**/*.svgicons/UserThin.svgis excluded by!**/*.svgicons/VerifiedSharpFilled.svgis excluded by!**/*.svgicons/Voltage.svgis excluded by!**/*.svgicons/Vote.svgis excluded by!**/*.svgicons/Wallet.svgis excluded by!**/*.svgicons/WalletThin.svgis excluded by!**/*.svgicons/Warning.svgis excluded by!**/*.svgicons/WarningFilled.svgis excluded by!**/*.svgicons/World.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.github/workflows/pull-request.yml.nvmrcAGENTS.mdCLAUDE.mdlintIcons.jslintIcons.test.mjspackage.json
evilpeach
left a comment
There was a problem hiding this comment.
should we still have the CONTRIBUTION.md?
# Contributing
See [AGENTS.md](AGENTS.md) for icon contribution and release guidelines.
Reject XML metadata outside allowed SVG content. Keep CONTRIBUTING.md as GitHub's contributor entry point.
|
Kept Implemented in commit |
Summary
Testing
Summary by CodeRabbit
New Features
Chores
Documentation