-
Notifications
You must be signed in to change notification settings - Fork 1.6k
🌱 chore: update the asciinema demo creation script #5068
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
base: master
Are you sure you want to change the base?
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wazery The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Hi @wazery. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
ce9b9ee to
ada0506
Compare
b6ef9e6 to
2f5bdbe
Compare
b339d16 to
0e9254a
Compare
| @@ -0,0 +1,108 @@ | |||
| #!/usr/bin/env bash | |||
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.
I checked the Makefile if we have some targets that just creates the needed kind cluster for testing, but I saw it's tightly coupled in the test scripts, so thought of introducing this minimal script just for the demo.
I feel my reasoning is not good here, and it would be nicer if I just decoupled the kind cluster creation from the test scripts, yet it's outside the scope of this PR 🤔 .. just thinking outloud 😅
0e9254a to
ee1f601
Compare
| return 0 | ||
| } | ||
|
|
||
| update_readme() { |
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 script should allow us update other videos such as the example shared so that we could add other demos
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.
addressed this, thanks for the pointer @camilamacedo86
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 enhances the asciinema demo recording infrastructure for Kubebuilder by introducing automated demo generation capabilities, a Kind cluster setup script, and Makefile targets for easier workflow management.
Key Changes:
- Added automated demo recording and SVG generation pipeline
- Introduced Kind cluster setup automation for consistent demo environments
- Enhanced demo script with modern Kubebuilder features (deploy-image plugin, validation markers)
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/demo/setup-kind.sh | New script that automates Kind cluster setup with installation checks and cluster configuration |
| scripts/demo/run.sh | Updated demo workflow showcasing modern features like deploy-image plugin and validation markers |
| scripts/demo/generate-demo.sh | New orchestration script that automates recording, SVG conversion, and README updates |
| scripts/demo/README.md | Comprehensive documentation update with new targets, prerequisites, and workflow instructions |
| Makefile | Added three new targets (update-demo, setup-demo-cluster, clean-demo) for demo management |
Comments suppressed due to low confidence (1)
scripts/demo/setup-kind.sh:1
- The
uname -mcommands should be quoted to handle edge cases with spaces or special characters. Use[ "$(uname -m)" = x86_64 ]instead.
#!/usr/bin/env bash
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fix kubectl apply command: change -k to -f for single YAML file - Extract duplicate version logic to main() function - Remove OS-specific installation commands from prerequisites - Simplify check_prerequisites error messages Addresses feedback from PR kubernetes-sigs#5068
ee1f601 to
59e50f0
Compare
- Revert to CronJob example to maintain consistency with existing demos - Make generate-demo.sh flexible to support multiple demo variations - Add support for custom demo names and scripts via CLI parameters - Only update README for default kb-demo, not custom demos - Add usage/help documentation for the script This addresses feedback from camilamacedo86 on PR kubernetes-sigs#5068: 1. Keep the same example (CronJob instead of Guestbook/Busybox) 2. Allow script to generate other demos for future extensibility
afbbbd1 to
e9a06b1
Compare
e9a06b1 to
b3ee602
Compare
This is a PR to address the issue #4552 which enhances the current script that creates the
asciinemademo recording, by having a minimal makefile target that generates the new recording and updates the root readme file.I also added a minimal script that sets up a minimal demo kind cluster so we can deploy into.