villebro commented on code in PR #34980:
URL: https://github.com/apache/superset/pull/34980#discussion_r2316580894
##########
.github/workflows/bashlib.sh:
##########
@@ -131,16 +96,11 @@ celery-worker() {
say "::endgroup::"
}
+# cypress-install() function - no-op, handled by setup-frontend action
cypress-install() {
- cd "$GITHUB_WORKSPACE/superset-frontend/cypress-base"
-
- cache-restore cypress
-
- say "::group::Install Cypress"
- npm ci
+ say "::group::cypress-install (no-op)"
+ echo "cypress installation handled by setup-frontend action - skipping"
say "::endgroup::"
-
- cache-save cypress
}
Review Comment:
Can't we just remove this if it's a no-op?
##########
.github/workflows/bashlib.sh:
##########
@@ -31,47 +31,12 @@ say() {
fi
}
-pip-upgrade() {
- say "::group::Upgrade pip"
- pip install --upgrade pip
- say "::endgroup::"
-}
-
-# prepare (lint and build) frontend code
-npm-install() {
- cd "$GITHUB_WORKSPACE/superset-frontend"
-
- # cache-restore npm
- say "::group::Install npm packages"
- echo "npm: $(npm --version)"
- echo "node: $(node --version)"
- npm ci
- say "::endgroup::"
+# npm-install() function removed - all workflows migrated to setup-frontend
action
Review Comment:
I assume this is AI being overly verbose, let's remove it
```suggestion
```
##########
.github/workflows/bashlib.sh:
##########
@@ -31,47 +31,12 @@ say() {
fi
}
-pip-upgrade() {
- say "::group::Upgrade pip"
- pip install --upgrade pip
- say "::endgroup::"
-}
-
-# prepare (lint and build) frontend code
-npm-install() {
- cd "$GITHUB_WORKSPACE/superset-frontend"
-
- # cache-restore npm
- say "::group::Install npm packages"
- echo "npm: $(npm --version)"
- echo "node: $(node --version)"
- npm ci
- say "::endgroup::"
+# npm-install() function removed - all workflows migrated to setup-frontend
action
- # cache-save npm
-}
-
-build-assets() {
- cd "$GITHUB_WORKSPACE/superset-frontend"
-
- say "::group::Build static assets"
- npm run build
- say "::endgroup::"
-}
-
-build-instrumented-assets() {
- cd "$GITHUB_WORKSPACE/superset-frontend"
-
- say "::group::Build static assets with JS instrumented for test coverage"
- cache-restore instrumented-assets
- if [[ -f "$ASSETS_MANIFEST" ]]; then
- echo 'Skip frontend build because instrumented static assets already
exist.'
- else
- npm run build-instrumented
- cache-save instrumented-assets
- fi
- say "::endgroup::"
-}
+# Frontend functions removed - migrated to setup-frontend action:
+# build-assets() → setup-frontend with build-assets: 'true'
+# build-instrumented-assets() → setup-frontend with build-instrumented: 'true'
+# cypress-install() → setup-frontend with install-cypress: 'true'
Review Comment:
same here
```suggestion
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
