Repository navigation
Add core git YAML functions - #2537
Conversation
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. |
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
|
Warning Release Documentation RequiredThis PR is labeled
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds pkg/git metadata helpers and YAML tag processors for !git.root, !git.sha, !git.ref, and !git.branch; registers tag constants and function handlers; routes config and exec YAML dispatch to the new processors; consolidates utils; and adds tests and documentation. ChangesGit YAML Function Tags
Sequence Diagram(s)sequenceDiagram
participant YAMLCaller as YAML processor / Function
participant TagProcessor as pkg/git.ProcessTag*
participant GitHelpers as pkg/git.GetRoot/GetCurrentCommitSHA/GetCurrentBranch
participant LocalRepo as Local Git Repository
participant Fallback as Provided default value
YAMLCaller->>TagProcessor: input tag string (e.g. "!git.ref <opt>")
TagProcessor->>TagProcessor: trimTagPrefix
TagProcessor->>GitHelpers: request requested metadata
GitHelpers->>LocalRepo: read HEAD/worktree
LocalRepo-->>GitHelpers: SHA / branch / root
alt success
GitHelpers-->>TagProcessor: resolved value
else failure
TagProcessor->>Fallback: extract supplied default
Fallback-->>TagProcessor: default or empty
end
TagProcessor-->>YAMLCaller: resolved or fallback result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
pkg/function/git_test.go (1)
32-54: ⚡ Quick winAdd the non-fallback counterpart for this recovery-path test.
This test verifies fallback activation outside a repository, but it should also include the opposite case (git context present) to assert fallback does not trigger when normal resolution succeeds.
As per coding guidelines: “Include negative-path tests for recovery logic: whenever a test verifies that a recovery/fallback triggers under condition X, add a corresponding test verifying the recovery does NOT trigger when condition X is absent”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/function/git_test.go` around lines 32 - 54, Add a complementary test that sets up a real git repository context and verifies the fallback does NOT run: create a new test (or extend TestGitFunctions_ExecuteWithFallbackOutsideRepository) that uses t.TempDir(), initializes a git repo (git init, create a commit, set a branch/ref), then call Execute on NewGitShaFunction, NewGitBranchFunction and NewGitRefFunction and assert the returned values are the actual git SHA/branch/ref (not the fallback strings like "unknown" or "detached"); use the same Execute(context.Background(), args, nil) invocation and require.NoError assertions to ensure normal resolution succeeds.pkg/utils/git.go (1)
36-37: ⚡ Quick winRefresh the
ProcessTagGitRootcomment to match the implementation.The comment says this uses go-git directly, but the function now delegates to
atmosGit.ProcessTagRoot.📝 Suggested comment fix
-// ProcessTagGitRoot returns the root directory of the Git repository using go-git. +// ProcessTagGitRoot returns the root directory of the current Git repository.As per coding guidelines: "Never delete existing comments without a very strong reason; preserve helpful comments explaining why/how/what/where, and update comments to match code when refactoring."
Also applies to: 46-46
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/utils/git.go` around lines 36 - 37, Update the doc comment for ProcessTagGitRoot to accurately describe its current behavior: instead of claiming it uses go-git directly, state that it delegates to atmosGit.ProcessTagRoot to obtain the repository root (and mention any relevant behavior such as input handling or error conditions inherited from atmosGit.ProcessTagRoot); do the same for the related comment at line 46 so comments reflect the refactor and delegation rather than the old implementation details.pkg/config/process_yaml.go (1)
332-337: ⚡ Quick winUpdate scalar-tag docs to include the new git tags.
The dispatch now supports
!git.rootalias,!git.sha/!git.ref, and!git.branch, but the function docs still describe the old tag set.📝 Suggested doc update
-// It supports the !env, !exec, !include, !repo-root, !cwd, and !random tags; failures during evaluation return an error wrapped with ErrExecuteYamlFunctions, and unknown/unsupported tags are decoded and returned as their YAML value. +// It supports !env, !exec, !include, !repo-root/!git.root, !git.sha/!git.ref, !git.branch, !cwd, and !random tags; failures during evaluation return an error wrapped with ErrExecuteYamlFunctions, and unknown/unsupported tags are decoded and returned as their YAML value. ... -// It dispatches handling for !env, !exec, !include, !repo-root, !cwd, and !random tags to their respective handlers. +// It dispatches handling for !env, !exec, !include, !repo-root/!git.root, !git.sha/!git.ref, !git.branch, !cwd, and !random tags to their respective handlers.As per coding guidelines: "Never delete existing comments without a very strong reason; preserve helpful comments explaining why/how/what/where, and update comments to match code when refactoring."
Also applies to: 367-372
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/config/process_yaml.go` around lines 332 - 337, Update the scalar-tag documentation/comments to reflect the new supported git tags and aliases: include !git.root (alias), !git.sha and !git.ref (both accepted), and !git.branch in the scalar-tag docs that currently describe the old tag set; specifically update the comment blocks that reference dispatching to processGitRootTag, processGitShaTag, and processGitBranchTag so their descriptions match the actual supported tags (also apply the same update to the other comment block near the second dispatch to these functions).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/function/git_root.go`:
- Line 44: Replace the direct fmt.Errorf wrapping on the git root failure with
the repository's static error from errors/errors.go (e.g., ErrGitRoot) and
combine it with the original err using errors.Join (or use fmt.Errorf with %w
only if you need a contextual message plus the static error); update the return
in pkg/function/git_root.go to return the empty string and
errors.Join(<staticErrorFromErrorsPackage>, err) and ensure the errors package
from errors/errors.go is imported.
In `@pkg/function/git.go`:
- Around line 37-40: Replace the ad-hoc fmt.Errorf wraps in pkg/function/git.go
(the ProcessTagSHA error and the two other fmt.Errorf sites) with the repository
static sentinel errors defined in errors/errors.go and combine them with the
underlying err using the repo pattern (errors.Join or fmt.Errorf with the
sentinel as the wrapped error per project style). Concretely, locate the failing
calls around atmosGit.ProcessTagSHA and the other two fmt.Errorf usages, import
the repo errors package and the standard errors package if needed, and return
the joined/static error (e.g., return "", errors.Join(errors.<SentinelError>,
err) or return "", fmt.Errorf("%w: additional context", errors.<SentinelError>))
instead of fmt.Errorf("...: %w", err).
In `@pkg/git/current_test.go`:
- Around line 93-95: The test uses a hardcoded Unix path "/fallback/root" when
calling ProcessTagRoot and asserting the result; replace the literal with a
cross-platform constructed path using filepath.Join (e.g. build the fallback arg
as YAMLFuncRoot + " " + filepath.Join("fallback","root") and assert.Equal
against that same filepath.Join value) and add the filepath import if missing so
the test is OS-agnostic; refer to ProcessTagRoot and YAMLFuncRoot to locate the
call and assertion.
In `@pkg/git/git.go`:
- Around line 214-222: GetRoot currently returns ad-hoc wrapped errors from
repo.Worktree() and filepath.Abs(...); replace those with the project's static
sentinel errors from errors/errors.go (e.g., ErrWorktreeNotFound /
ErrPathResolve or the appropriate named errors) by joining or wrapping the
original errors—use errors.Join(staticErr, err) or fmt.Errorf("context: %w",
staticErr) combined with %w for the original error per guideline—update the
error returns in the repo.Worktree() failure and the filepath.Abs(...) failure
sites so they consistently return the cataloged static errors plus the
underlying error.
- Around line 15-18: Remove the package-local sentinels ErrDetachedHead and
ErrEmptyBranchName from git.go and instead reference the centralized sentinel
errors you must add to the shared error catalog (define ErrDetachedHead and
ErrEmptyBranchName as exported variables in the shared errors package). Update
git.go to import that shared errors package and use the shared symbols (e.g.,
sharederrors.ErrDetachedHead, sharederrors.ErrEmptyBranchName) everywhere this
file currently creates or compares those errors so that callers can use
errors.Is() consistently.
---
Nitpick comments:
In `@pkg/config/process_yaml.go`:
- Around line 332-337: Update the scalar-tag documentation/comments to reflect
the new supported git tags and aliases: include !git.root (alias), !git.sha and
!git.ref (both accepted), and !git.branch in the scalar-tag docs that currently
describe the old tag set; specifically update the comment blocks that reference
dispatching to processGitRootTag, processGitShaTag, and processGitBranchTag so
their descriptions match the actual supported tags (also apply the same update
to the other comment block near the second dispatch to these functions).
In `@pkg/function/git_test.go`:
- Around line 32-54: Add a complementary test that sets up a real git repository
context and verifies the fallback does NOT run: create a new test (or extend
TestGitFunctions_ExecuteWithFallbackOutsideRepository) that uses t.TempDir(),
initializes a git repo (git init, create a commit, set a branch/ref), then call
Execute on NewGitShaFunction, NewGitBranchFunction and NewGitRefFunction and
assert the returned values are the actual git SHA/branch/ref (not the fallback
strings like "unknown" or "detached"); use the same
Execute(context.Background(), args, nil) invocation and require.NoError
assertions to ensure normal resolution succeeds.
In `@pkg/utils/git.go`:
- Around line 36-37: Update the doc comment for ProcessTagGitRoot to accurately
describe its current behavior: instead of claiming it uses go-git directly,
state that it delegates to atmosGit.ProcessTagRoot to obtain the repository root
(and mention any relevant behavior such as input handling or error conditions
inherited from atmosGit.ProcessTagRoot); do the same for the related comment at
line 46 so comments reflect the refactor and delegation rather than the old
implementation details.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 910bbab8-4c1b-49b6-a388-d49878d9abe3
📒 Files selected for processing (20)
internal/exec/yaml_func_utils.gointernal/exec/yaml_func_utils_context_test.gopkg/config/process_yaml.gopkg/config/process_yaml_test.gopkg/function/defaults.gopkg/function/git.gopkg/function/git_root.gopkg/function/git_root_test.gopkg/function/git_test.gopkg/function/registry_test.gopkg/function/tags.gopkg/function/tags_test.gopkg/git/current_test.gopkg/git/git.gopkg/git/yaml_tags.gopkg/provisioner/source/vendor_test.gopkg/utils/git.gopkg/utils/git_test.gopkg/utils/yaml_utils.gopkg/utils/yaml_utils_test.go
|
Actionable comments posted: 0 |
1 similar comment
|
Actionable comments posted: 0 |
|
Actionable comments posted: 0 |
|
Actionable comments posted: 0 |
|
Actionable comments posted: 0 |
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (71.64%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2537 +/- ##
==========================================
+ Coverage 78.58% 78.60% +0.02%
==========================================
Files 1145 1147 +2
Lines 110306 110458 +152
==========================================
+ Hits 86685 86831 +146
+ Misses 18813 18806 -7
- Partials 4808 4821 +13
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. |
|
These changes were released in v1.221.0-rc.1. |
what
!git.root,!git.sha,!git.branch, and!git.ref.pkg/git, withpkg/utilslimited to compatibility shims and YAML tag registration.why
!git.ref.pkg/utilsby placing Git behavior in the self-contained Git package.references
Summary by CodeRabbit
New Features
Refactor
Tests
Documentation