-
Notifications
You must be signed in to change notification settings - Fork 82
refactor env editor #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Zzde <[email protected]>
Signed-off-by: Zzde <[email protected]>
Signed-off-by: Zzde <[email protected]>
Signed-off-by: Zzde <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the environment editor by adding dedicated selectors for secrets and configmaps, cleaning up build scripts, and improving CI/CD workflows.
- Introduces reusable UI components to select Kubernetes secrets and configmaps
- Moves Prettier to devDependencies and tidies the Makefile
- Updates terminal error handling and enhances Docker build & CI workflows
Reviewed Changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
ui/src/components/secret-selector.tsx | Add SecretSelector component for choosing Kubernetes secrets |
ui/src/components/configmap-selector.tsx | Add ConfigMapSelector component for choosing Kubernetes configmaps |
ui/package.json | Relocate prettier to devDependencies |
pkg/kube/terminal.go | Replace server log with client-facing error via SendErrorMessage |
Makefile | Remove static dir variable, adjust lint target |
.github/workflows/push.yaml | Enable multi-arch builds in Docker action |
.github/workflows/ci.yml | Add new CI workflow (build, lint, test) |
Files not reviewed (1)
- ui/pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
.github/workflows/ci.yml:49
- The CI
Test
step invokes the help flag instead of running actual tests. Replace this with real test commands (e.g.,go test ./...
) to ensure coverage.
run: ./kite -h
kill $$BACKEND_PID 2>/dev/null | ||
|
||
lint: ## Run linters | ||
lint: golangci-lint ## Run linters |
Copilot
AI
Jun 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lint
target now depends on a golangci-lint
rule which isn't defined in the Makefile, causing make lint
to fail. Add a golangci-lint
target or remove this dependency.
Copilot uses AI. Check for mistakes.
TerminalSizeQueue: session, | ||
}) | ||
|
||
if err != nil { |
Copilot
AI
Jun 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Replacing the server-side klog.Errorf
with only a client error message may reduce observability. Consider retaining a klog.Errorf
call alongside SendErrorMessage
.
if err != nil { | |
if err != nil { | |
klog.Errorf("Error during exec stream for pod %s in namespace %s: %v", session.podName, session.namespace, err) |
Copilot uses AI. Check for mistakes.
}) { | ||
const { data, isLoading } = useResources('configmaps', namespace) | ||
|
||
const sortedConfigMaps = useMemo(() => { |
Copilot
AI
Jun 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The sorting and rendering logic here duplicates the one in SecretSelector
. Consider extracting a generic selector component to avoid code duplication.
Copilot uses AI. Check for mistakes.
No description provided.