SEZ9 commented on PR #10973: URL: https://github.com/apache/seatunnel/pull/10973#issuecomment-5564479007
Thanks @DanielLeens for re-reviewing `2f9f7341693` from scratch and tracing the delta from `956593c7eec` file by file. Agreed on the two items you walked through: - **Issue 1 (`date-fns`/`date-fns-tz`)** — framing this as a correctness fix rather than a size win is right, and it's good that, as you note, the PR description now carries the revised ~53 MB figure instead of ~90 MB. - **Issue 2 (Ionicons attribution)** — the in-file MIT header in `src/components/icons/index.ts` pointing to `seatunnel-dist/release-docs/licenses/LICENSE-ionicons.txt` is exactly what was asked for, so that closes it from my side too. One note: your comment appears to be cut off mid-header (it ends at `+ * Copyright`), so I can't see the part covering the mirror-registry lockfile and the PR-description claim you list as fixed. Could you repost the rest, or confirm the following so we can wrap up: 1. **Registry** — every entry in the regenerated `package-lock.json` now points at the official npm registry, with none at the third-party mirror. 2. **`--omit=dev` claim** — the PR description no longer suggests `npm install --omit=dev` as a CI optimisation, since the build tooling now lives in `devDependencies`. 3. **Vendored SVG** — a one-line confirmation that the vendored icon data is path data only (no scripts or event handlers). 4. **Native watcher pulled in by plain `sass`** — has the install been exercised with `--ignore-scripts`? If the native install script is a problem on locked-down CI, better to know before merge. 5. **Leftover protobuf lock entry** — a short note in the PR description that it is now marked optional/peer after the `sass-embedded` removal would help future readers of the lockfile diff. None of these are source-level blockers given what you've verified; they're confirmations so the record on this PR is complete. Once 1 and 2 are confirmed I'm comfortable moving forward. <!-- streview-comment:860 --> -- 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]
