SEZ9 commented on PR #10973: URL: https://github.com/apache/seatunnel/pull/10973#issuecomment-6008529533
Thanks for posting the snippets from the current head. On the three medium items — the Ionicons attribution in `src/components/icons/index.ts`, the registry used in `seatunnel-engine/seatunnel-engine-ui/package-lock.json`, and the `npm install --omit=dev` claim in the PR description — the quoted excerpts look like the right fixes. I'd like to close them against the PR diff itself rather than quoted text, so could you point me to those changes in the diff? Since the head moved from `2f9f7341693` to `cf056580c1d1` via rebase, please also confirm there is no content change between the two. Once I see them in the diff I'll mark all three resolved. Still open from the earlier review: 1. **Vendored SVG file**: a one-line confirmation that it contains only path data and no executable code is enough. 2. **Optional native watcher pulled in by `sass`** (the parcel watcher package, which ships platform binaries with an install script): please verify a clean install with scripts disabled still builds, and note the actual on-disk size delta so the savings figure in the description is accurate. 3. **bufbuild protobuf lock entry** now marked `optional`/`peer` after the sass-embedded removal: either drop it if nothing depends on it, or add a one-line note to the PR description explaining why it stays. Once those are answered I'm happy to approve. <!-- streview-comment:1547 --> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
