SEZ9 commented on PR #10973: URL: https://github.com/apache/seatunnel/pull/10973#issuecomment-5385644837
@DanielLeens Thanks for re-verifying the three Medium findings against head `df6e9a45708b` — I agree with your updated position that Issues 1–3 are must-fix before merge: 1. **In-file MIT attribution** — `src/components/icons/index.ts` needs the Ionicons copyright/permission notice co-located with the vendored SVG data; the `seatunnel-dist/release-docs/LICENSE` entry alone doesn't satisfy the policy. 2. **Mirror-resolved lockfile** — the lockfile should be regenerated so `resolved` entries point at the official npm registry rather than `registry.npmmirror.com`. Your count (149 mirror entries, zero official) shows this PR expands that trust dependency, so fixing it here makes sense. 3. **`--omit=dev` claim** — the PR description should be corrected: with `sass`, `tailwindcss`, `postcss`, and `autoprefixer` in `devDependencies` and all needed by `vite build`, `--omit=dev` would break the build. The dependency reclassification itself is fine. Agreed that Issues 4–8 remain non-blocking as scored. Once a new commit addresses 1–3, a focused re-review on those three points sounds like the right next step. Thanks again for the thorough cross-check. <!-- streview-comment:476 --> -- 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]
