DanielLeens commented on PR #10973: URL: https://github.com/apache/seatunnel/pull/10973#issuecomment-5369020685
@SEZ9 Thanks for the deep pass on this — I independently re-verified the three Medium-severity findings against the actual head `df6e9a45708b` before replying, since they directly contradict my own "no source-level blocker" conclusion from the same head: 1. **Issue 1 (missing in-file MIT attribution)** — confirmed. `src/components/icons/index.ts` only carries the standard ASF header; there's no Ionicons copyright/permission notice in the file itself, only the `seatunnel-dist/release-docs/LICENSE` entry. You're right that ASF third-party policy wants the notice co-located with the copied content, not only in the distribution LICENSE. I missed this — I checked the LICENSE/licenses/ bookkeeping but didn't check for in-file attribution of the vendored SVG data. 2. **Issue 2 (package-lock.json resolving to registry.npmmirror.com)** — confirmed by direct count: 149 `resolved` entries in this PR's lockfile diff point at `registry.npmmirror.com`, zero point at `registry.npmjs.org`. Agreed this shouldn't be cemented further in an ASF release artifact, especially since this PR already rewrites a large chunk of the lock. 3. **Issue 3 (`--omit=dev` guidance is wrong)** — confirmed. The PR body does say moving `tailwindcss`/`postcss`/`autoprefixer` to `devDependencies` "enables future CI optimisation with `npm install --omit=dev`", but `sass` is in `devDependencies` too now and `vite build` needs all four at build time — `--omit=dev` would break the build, not optimize it. That's a real, actionable correction to the PR description, separate from the dependency reclassification itself (which is fine). So: my prior "Ready to merge after fixes / no blockers" conclusion on this head was wrong on the licensing-attribution point specifically, and the CI-optimization claim needs correcting regardless of merge readiness. Updating my position — Issues 1–3 should be treated as must-fix before merge (attribution is a real ASF-policy gap, not style), Issues 4–8 remain useful but non-blocking as you scored them. @davidzollo once Issues 1–3 are addressed I'm happy to do another pass focused specifically on those three. -- 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]
