SEZ9 commented on PR #10973: URL: https://github.com/apache/seatunnel/pull/10973#issuecomment-5747468876
Thanks for working through the follow-ups on this one. Where things stand from my side on the earlier review points: - Protobuf lock entry / `sass-embedded` as `optional`/`peer`: the PR description now explains this, so that point is closed. - Native watcher install script: a non-blocking note about `--ignore-scripts` for locked-down deployments is sufficient; no dedicated CI step needed. - The unrelated CI failures were re-run and `Build` is green on the current head (`2f9f73416937`), with no new commits since. Before I approve, I'd like to confirm the remaining earlier items directly on `2f9f73416937`, since the thread only summarises them as confirmed without the details: 1. **Ionicons attribution** – please point me at the in-file MIT attribution comment for the copied SVG path data in `src/components/icons/index.ts` (the release-docs LICENSE entry alone isn't sufficient for a vendored snippet). 2. **Lockfile registry** – please confirm the new entries in `seatunnel-engine/seatunnel-engine-ui/package-lock.json` resolve to the official npm registry rather than `registry.npmmirror.com`. 3. **`npm install --omit=dev`** – please confirm the PR description no longer suggests this as a CI optimisation, since postcss/autoprefixer/tailwindcss/sass moved to devDependencies and the build needs them. A short reply with the relevant snippets is enough; once those three are confirmed I'm happy to approve and merge. <!-- streview-comment:1185 --> -- 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]
