MartijnVisser commented on PR #28710: URL: https://github.com/apache/flink/pull/28710#issuecomment-4984311890
Thanks for the PR @spuru9! I went through the diff. The tooling bumps are all `devDependencies`, the `dependencies` block is untouched, and none of prettier/stylelint end up in the served bundle, so NOTICE correctly needs no change here. The mechanical `lint:fix` output checks out too: the `inset: 0` collapses are equivalent to the four `top`/`right`/`bottom`/`left: 0` lines (only the all-zero blocks collapsed, the asymmetric ones were left alone), the dagre `background` rewrap is cosmetic with the value and its inline comments unchanged, and the `task-manager-metrics` interpolation rewrap renders identically. Dropping `stylelint-config-prettier` is safe: it's unreferenced, and it's a different package from the `eslint-config-prettier` we still rely on. Two things worth fixing: 1. The description says the reformat was committed "as a separate, isolated commit for easy review", but there's only one commit here that combines the tooling bump with the `lint:fix` output. Could you either split it into two (the bump + `.stylelintrc.js` config, then the mechanical reformat) or update the description to match? The split would make the reformat easier to skim past in review. 2. `color-function-alias-notation` is disabled outright. The alpha-channel trap you found is real, but `'legacy'` keeps the rule on while pinning to the Less-safe `rgba()` alias, so we don't lose the guard entirely. Could you try that and confirm it doesn't flag the existing `rgba()` usages (`app.component.less`, `dagre.component.less`, `styles/index.less`)? If it misfires on Less syntax the way the two `declaration-property-value-*` rules do, then leaving it null is fine and let's keep it. `import-notation: 'string'` is a nice touch, that's the right way to stop the `@import url()` rewrite without losing the consistency check. I'll do a local browser smoke of the log/stdout/thread-dump and log-detail full-screen panes (the `inset` changes) plus the TaskManager metrics table before merging, since those are the parts CI can't catch. -- 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]
