-
Notifications
You must be signed in to change notification settings - Fork 0
Update cli-minikit.yml #49
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: Dargon789 <64915515+Dargon789@users.noreply.github.com>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideResolves a merge conflict in the cli-minikit GitHub Actions workflow by standardizing the workflow name and the CLI invocation used to generate a minikit test project, while leaving the install/build steps unchanged. Flow diagram for updated cli-minikit GitHub Actions workflowflowchart TD
A[GitHub pull_request event on main] --> B[Workflow cli_build_install_minikit]
B --> C[Job cli-minikit runs on ubuntu-latest]
C --> D[Checkout repository actions_checkout]
D --> E[Setup Node.js actions_setup-node]
E --> F[Install root dependencies
npm install]
F --> G[Create test project directory
mkdir test-project
cd test-project]
G --> H[Generate minikit test project
create-onchain cli.js --mini
with scripted stdin]
H --> I[Install test project dependencies
npm install in test-project/my-onchainkit-app]
I --> J[Build test project
npm run build]
J --> K[Job success / failure reported back to pull_request]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
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.
Hey there - I've reviewed your changes - here's some feedback:
- The updated workflow name and CLI invocation semantics have changed from the previous onchainkit variant; if this is intended to supersede that behavior, consider aligning or clearly differentiating naming/flags so it’s obvious which workflow covers which use case.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The updated workflow name and CLI invocation semantics have changed from the previous onchainkit variant; if this is intended to supersede that behavior, consider aligning or clearly differentiating naming/flags so it’s obvious which workflow covers which use case.
## Individual Comments
### Comment 1
<location> `.github/workflows/cli-minikit.yml:54` </location>
<code_context>
cd test-project
-<<<<<<<< HEAD:.github/workflows/cli-minikit.yml
+
(sleep 1; echo ""; sleep 1; echo ""; sleep 1; echo -e "\033[D\n"; sleep 1; echo -e "\033[D\n") | ../packages/create-onchain/dist/esm/cli.js --mini
-========
- (sleep 1; echo ""; sleep 1; echo ""; sleep 1; echo ""; sleep 1; echo -e "\033[D\n") | ../packages/create-onchain/dist/esm/cli.js
</code_context>
<issue_to_address>
**suggestion (testing):** The CLI interaction script using sleeps and escape codes is fairly brittle and might be prone to flakiness.
Using chained `sleep`/`echo` with cursor escape codes ties this step to specific timing and TTY behavior on the GitHub runner, which is likely to be flaky. Prefer a more deterministic input mechanism (e.g., here-doc, `printf`, or CLI flags/env vars if available) so the interaction is stable and easier to maintain.
Suggested implementation:
```
cd test-project
# Provide deterministic stdin to the CLI instead of relying on timing/cursor escape sequences.
# Adjust the number/content of newlines if the CLI’s prompts change.
printf '\n\n\n\n' | ../packages/create-onchain/dist/esm/cli.js --mini
```
If the CLI expects specific answers (not just “enter to accept default”), replace the `printf '\n\n\n\n'` with the exact answers, one per line, for example:
`printf 'my-app-name\noption-1\n\nfinal-answer\n' | ...`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
What changed? Why?
Notes to reviewers
How has it been tested?
Summary by Sourcery
CI: