chore: workshop setup - #2409
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a Workshop configuration for charmhub.io to standardize local dev setup (Python via uv, Node via node SDK) and provide common run/build/test/lint actions matching the existing dotrun-based workflow.
Changes:
- Introduces
workshop.yamlwith SDK definitions, tunnel wiring, and dev/build/test/lint actions. - Adds
.workshop/charmhub-ioscripts/hooks to install dependencies, set environment, and report health status. - Updates
.gitignoreto ignore the Workshop lock file.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| workshop.yaml | Defines the Workshop environment, tunnels, and common actions (start/build/test/lint, etc.). |
| .workshop/charmhub-io/sdk.yaml | Declares the repo-specific Workshop SDK name. |
| .workshop/charmhub-io/install.sh | Installs Python deps into .venv via uv and runs yarn install. |
| .workshop/charmhub-io/hooks/setup-project | Runs install and appends env sourcing to ~/.profile for interactive shells. |
| .workshop/charmhub-io/hooks/setup-base | Installs base apt packages needed for builds and Cypress/headless runs. |
| .workshop/charmhub-io/hooks/check-health | Marks Workshop health based on venv + node_modules presence. |
| .workshop/charmhub-io/env.sh | Shared env loader (.env/.env.local) and PATH/corepack-yarn shim logic. |
| .gitignore | Ignores .workshop.lock. |
Suppressed comments (2)
workshop.yaml:63
- These actions use
source, which is not POSIX and may fail ifworkshop runexecutes actions under/bin/sh. Prefer. /project/.workshop/charmhub-io/env.shfor portability.
test: |
source /project/.workshop/charmhub-io/env.sh
yarn test
# Cypress needs a display; CI gets one from a container, here xvfb provides it.
test-e2e: |
source /project/.workshop/charmhub-io/env.sh
cypress install
xvfb-run -a yarn test-e2e
workshop.yaml:76
- These actions also use
source(bash-only). Switching to the POSIX.builtin avoids depending on the action runner shell being bash.
lint: |
source /project/.workshop/charmhub-io/env.sh
yarn lint-python
yarn lint-js
yarn lint-scss
# Escape hatch for any other package.json script:
# workshop run yarn test-js / test-python / format-python / serve
yarn: |
source /project/.workshop/charmhub-io/env.sh
yarn "$@"
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # Native build deps for the Python/Node dependency trees. | ||
| # Evidence: Dockerfile python-dependencies stage. | ||
| apt-get update | ||
| apt-get install --no-install-recommends --yes \ |
There was a problem hiding this comment.
I don't think we actually need these, they're not even real dependencies for the project... on snapcraft this step is empty and it works fine.
Also, why these are in the Dockerfile is a mystery to me
There was a problem hiding this comment.
cool! I didn't know about this hook
| source /project/.workshop/charmhub-io/env.sh | ||
| yarn test | ||
|
|
||
| # One intent, three linters. |
There was a problem hiding this comment.
absolute slop comment lmao
| actions: | ||
| # Equivalent of the old `dotrun`: Vite + gunicorn together on 8045/5045. | ||
| start: | | ||
| source /project/.workshop/charmhub-io/env.sh |
There was a problem hiding this comment.
there's no need to explicitly source the script here (or any of the following actions), it's sourced through ~/.profile
| yarn build | ||
| ENVIRONMENT=prod yarn serve | ||
|
|
||
| install: | |
There was a problem hiding this comment.
good point, I forgot dotrun had clean and install commands, I should add them to the snapcraft workshop as well
There was a problem hiding this comment.
lots of noise in this file... it can be cleaned up a lot:
- this script always runs in
/projectas the workshop user both duringsetup-project(confirmed by docs) and when called as an action, so all the explicit paths can be removed - no need for that
! -xguard on the venv python, justuv venv --allow-existing - sourcing the env does nothing in this step (unless we're doing stuff like override node, uv or python env vars and I don't think we are)
| if ! command -v yarn >/dev/null 2>&1; then | ||
| CHARMHUB_IO_SHIM_DIR="${XDG_CACHE_HOME:-$HOME/.cache}/charmhub-io-workshop/bin" | ||
| mkdir -p "$CHARMHUB_IO_SHIM_DIR" | ||
| if [ ! -x "$CHARMHUB_IO_SHIM_DIR/yarn" ]; then | ||
| printf '#!/bin/bash\nexec corepack yarn "$@"\n' > "$CHARMHUB_IO_SHIM_DIR/yarn" | ||
| chmod +x "$CHARMHUB_IO_SHIM_DIR/yarn" | ||
| fi | ||
| export PATH="$CHARMHUB_IO_SHIM_DIR:$PATH" | ||
| fi |
There was a problem hiding this comment.
why would the yarn shim not be in PATH?
|
|
||
| # Sourced by the actions AND by ~/.profile, so guard against PATH entries | ||
| # piling up in nested shells. | ||
| for _charmhub_dir in "$PROJECT_DIR/.venv/bin" "$PROJECT_DIR/node_modules/.bin"; do |
There was a problem hiding this comment.
why not activate the .venv normally? also I thought adding node_modules/.bin to PATH was bad form?
|
Done
How to QA