docs(starters): gate function guidance in base scaffold README#12
Open
tkkhq wants to merge 2 commits into
Open
docs(starters): gate function guidance in base scaffold README#12tkkhq wants to merge 2 commits into
tkkhq wants to merge 2 commits into
Conversation
The base starter is applied on every volcano init, so its README is shared across base-only and template scaffolds. It described the directory as containing "configuration, functions" and unconditionally listed volcano functions deploy --all / cloud functions deploy --all, which is misleading for bare volcano init (no functions or config are created). Gate the function deploy guidance behind "If this project includes volcano/functions:" — the same conditional already used for config — so the README stays accurate for both base-only and template scaffolds. Update the contents line to list only what base actually creates (migrations and local variables), and add a test assertion to lock in the gated guidance. Addresses review feedback on #9.
There was a problem hiding this comment.
Pull request overview
Updates the base starter scaffold README so it remains accurate for both base-only volcano init projects and template-based scaffolds, particularly by avoiding misleading “deploy functions” guidance when no functions exist.
Changes:
- Adjusts the base scaffold
volcano/README.mddescription to reflect what base actually creates (migrations + local variables). - Gates functions deploy instructions behind a “If this project includes volcano/functions:” condition (matching the existing conditional style for
volcano-config.yaml). - Extends
projectinittests to assert the updated README content (including removal of the stale “configuration, functions, migrations” phrase).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/projectinit/starters/base/volcano/README.md | Updates base scaffold documentation to avoid unconditional functions guidance and improve section structure. |
| internal/projectinit/projectinit_test.go | Adds assertions to ensure the generated base README contains the new gated functions guidance and removes stale text. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+41
to
+43
| assert.Contains(t, readme, "If this project includes volcano/volcano-config.yaml") | ||
| assert.Contains(t, readme, "If this project includes volcano/functions:") | ||
| assert.NotContains(t, readme, "configuration, functions, migrations") |
Contributor
Author
There was a problem hiding this comment.
Good catch — addressed in c5a15ec. The heading-only Contains checks are replaced with structural assertions that verify each deploy command appears directly under its gating heading, plus strings.Count == 1 checks so reintroducing an unconditional copy (the exact failure mode raised) now fails the test. Applied symmetrically to both functions and config, local + cloud. Also restored the README to main's unfenced style to keep this PR's diff limited to the gating change.
Address Copilot review on #12: the prior Contains checks only asserted the gating headings existed, not that the deploy commands sit behind them. Assert each function/config command appears under its gating heading (local + cloud) and that each command occurs exactly once, so reintroducing an unconditional copy would fail the test. Restore the base README to the unfenced style used on main; the prior commit had inadvertently rewritten the whole file with fenced code blocks and ## headings, adding unrelated reformatting noise to a gating-only change.
Contributor
Author
|
Replying to @copilot-pull-request-reviewer[bot]'s review: thanks — addressed in c5a15ec.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tracking
Problem
The generated
internal/projectinit/starters/base/volcano/README.mdis shipped with everyvolcano init(thebasestarter is always applied first), so it is shared across base-only and template scaffolds. It described the directory as containing "configuration, functions" and unconditionally told users to runvolcano functions deploy --all/volcano cloud functions deploy --all— misleading for barevolcano init, which creates no functions or config.Changes
migrations and local variables.If this project includes volcano/functions:— the same conditional pattern already used for config — so the README stays accurate for both base-only and template scaffolds.Verification
make lintgo test ./...go build ./...go vet ./...