fix: Improve kpt version command output and add comprehensive tests#4465
fix: Improve kpt version command output and add comprehensive tests#4465NETIZEN-11 wants to merge 1 commit intokptdev:mainfrom
Conversation
NETIZEN-11
requested review from
kispaljr,
liamfallon and
mozesl-nokia
as code owners
✅ Deploy Preview for kptdocs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
dosubot
bot
added
size:XXL
There was a problem hiding this comment.
Copilot reviewed 17 out of 18 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // setRenderStatus reads the Kptfile at pkgPath, sets the Rendered condition and RenderStatus, and writes it back. | ||
| func setRenderStatus(fs filesys.FileSystem, pkgPath string, condition kptfilev1.Condition, renderStatus *kptfilev1.RenderStatus) { | ||
| fsOrDisk := filesys.FileSystemOrOnDisk{FileSystem: fs} | ||
| kf, err := kptfileutil.ReadKptfile(fsOrDisk, pkgPath) | ||
| if err != nil { | ||
| klog.V(3).Infof("failed to read Kptfile for render status update at %s: %v", pkgPath, err) | ||
| return | ||
| } | ||
| if kf.Status == nil { | ||
| kf.Status = &kptfilev1.Status{} | ||
| } | ||
| // Replace any existing Rendered condition | ||
| kf.Status.Conditions = slices.DeleteFunc(kf.Status.Conditions, func(c kptfilev1.Condition) bool { | ||
| return c.Type == kptfilev1.ConditionTypeRendered | ||
| }) | ||
| kf.Status.Conditions = append(kf.Status.Conditions, condition) | ||
| kf.Status.RenderStatus = renderStatus | ||
|
|
||
| // Update render status if provided | ||
| if renderStatus != nil { | ||
| kf.Status.RenderStatus = renderStatus | ||
| } | ||
|
|
||
| if err := kptfileutil.WriteKptfileToFS(fs, pkgPath, kf); err != nil { | ||
| klog.V(3).Infof("failed to write render status to Kptfile at %s: %v", pkgPath, err) | ||
| } | ||
| } |
There was a problem hiding this comment.
The function setRenderStatus (lines 322-347) appears to be unused dead code. It is identical to setRenderConditionWithStatus which is called from updateRenderStatus. Consider removing this function to reduce code duplication and maintenance burden.
internal/builtins/pkg_context.go
Outdated
| // isRootKptfile checks if the given path represents a root Kptfile | ||
| func isRootKptfile(kptfilePath string) bool { | ||
| cleanPath := path.Clean(kptfilePath) | ||
| base := path.Base(cleanPath) | ||
| return base == kptfilev1.KptFileGVK().Kind | ||
| } |
There was a problem hiding this comment.
The function isRootKptfile() name is misleading - it only checks if the filename is "Kptfile", not whether the Kptfile is actually at the root level. The actual root check is done separately at line 135. Consider renaming this function to isKptfile() or combining the two checks into a single function for clarity.
| if kf == nil { | ||
| return fmt.Errorf("kptfile cannot be nil") | ||
| } | ||
|
|
There was a problem hiding this comment.
The performUpdate function accesses kf.Upstream.Type at line 136 without checking if kf.Upstream is nil. Although loadAndValidateKptfile ensures this is non-nil in the current call path, performUpdate should add a defensive nil check for better robustness, especially since it could be called from other contexts in the future.
| if kf.Upstream == nil { | |
| return fmt.Errorf("package must have an upstream reference") | |
| } |
- Update version command to display semantic version format clearly - Add proper handling for unknown/development builds - Include comprehensive test coverage for version command - Ensure consistent version output across all architectures Part of kptdev#4450 - CLI Version Fix
NETIZEN-11
force-pushed
the
feat/issue-4450-pr2-cli-version
branch
from
82c8cc4 to
e87a8da
Compare
dosubot
bot
added
size:L
Description
This PR improves the
kpt versioncommand to provide clear, consistent semantic version output across all architectures and adds comprehensive test coverage.Part of #4450 - Stabilize kpt API to version 1
Changes Made
Version Command Enhancement (
run/run.go)Test Coverage (
run/run_test.go)Testing