Skip to content
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

remove unnecessary push when rendering to local path #272

Merged
merged 1 commit into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 27 additions & 6 deletions branches.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,12 @@
logger := rc.logger.WithField("targetBranch", rc.request.TargetBranch)

// Check if the target branch exists on the remote
targetBranchExists, err := rc.repo.RemoteBranchExists(rc.request.TargetBranch)
remoteTargetBranchExists, err := rc.repo.RemoteBranchExists(rc.request.TargetBranch)

Check warning on line 83 in branches.go

View check run for this annotation

Codecov / codecov/patch

branches.go#L83

Added line #L83 was not covered by tests
if err != nil {
return fmt.Errorf("error checking for existence of target branch: %w", err)
return fmt.Errorf("error checking for existence of remote target branch: %w", err)

Check warning on line 85 in branches.go

View check run for this annotation

Codecov / codecov/patch

branches.go#L85

Added line #L85 was not covered by tests
}

if targetBranchExists {
if remoteTargetBranchExists {

Check warning on line 88 in branches.go

View check run for this annotation

Codecov / codecov/patch

branches.go#L88

Added line #L88 was not covered by tests
logger.Debug("target branch exists on remote")
if err = rc.repo.Fetch(); err != nil {
return fmt.Errorf("error fetching from remote: %w", err)
Expand All @@ -103,10 +103,31 @@
}

logger.Debug("target branch does not exist on remote")
if err = rc.repo.CreateOrphanedBranch(rc.request.TargetBranch); err != nil {
return fmt.Errorf("error creating new target branch: %w", err)

// Check if the target branch exists locally
localTargetBranchExists, err := rc.repo.LocalBranchExists(rc.request.TargetBranch)
if err != nil {
return fmt.Errorf("error checking for existence of local target branch: %w", err)

Check warning on line 110 in branches.go

View check run for this annotation

Codecov / codecov/patch

branches.go#L106-L110

Added lines #L106 - L110 were not covered by tests
}
logger.Debug("created target branch")

if localTargetBranchExists {
logger.Debug("target branch exists locally")
if err = rc.repo.Checkout(rc.request.TargetBranch); err != nil {
return fmt.Errorf("error checking out target branch: %w", err)
}
logger.Debug("checked out target branch")
} else {
Comment on lines +113 to +119
Copy link
Member Author

@krancour krancour Mar 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added for good measure; instead of blindly creating an orphan branch.

logger.Debug("target branch does not exist locally")
if err = rc.repo.CreateOrphanedBranch(rc.request.TargetBranch); err != nil {
return fmt.Errorf("error creating new target branch: %w", err)
}
logger.Debug("created target branch locally")

Check warning on line 124 in branches.go

View check run for this annotation

Codecov / codecov/patch

branches.go#L113-L124

Added lines #L113 - L124 were not covered by tests
}

if rc.request.LocalOutPath != "" {
return nil // There's no need to push the new branch to the remote
}

Check warning on line 129 in branches.go

View check run for this annotation

Codecov / codecov/patch

branches.go#L127-L129

Added lines #L127 - L129 were not covered by tests
Comment on lines +127 to +129
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the important bit.


if err = rc.repo.Commit(
"Initial commit",
&git.CommitOptions{
Expand Down
2 changes: 1 addition & 1 deletion docs/docs/30-how-to-guides/30-docker-image.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ easiest option for experimenting locally with Kargo Render!
Example usage:

```shell
docker run -it ghcr.io/akuity/kargo-render:v0.1.0-rc.38 \
docker run -it ghcr.io/akuity/kargo-render:v0.1.0-rc.39 \
--repo https://github.com/<your GitHub handle>/kargo-render-demo-deploy \
--repo-username <your GitHub handle> \
--repo-password <a GitHub personal access token> \
Expand Down
17 changes: 17 additions & 0 deletions pkg/git/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@
// LastCommitID returns the ID (sha) of the most recent commit to the current
// branch.
LastCommitID() (string, error)
// LocalBranchExists returns a bool indicating if the specified branch exists.
LocalBranchExists(branch string) (bool, error)
// CommitMessage returns the text of the most recent commit message associated
// with the specified commit ID.
CommitMessage(id string) (string, error)
Expand Down Expand Up @@ -366,6 +368,21 @@
return strings.TrimSpace(string(shaBytes)), nil
}

func (r *repo) LocalBranchExists(branch string) (bool, error) {
resBytes, err := libExec.Exec(r.buildCommand(
"branch",
"--list",
branch,
))
if err != nil {
return false,
fmt.Errorf("error checking for existence of local branch %q: %w", branch, err)
}

Check warning on line 380 in pkg/git/git.go

View check run for this annotation

Codecov / codecov/patch

pkg/git/git.go#L378-L380

Added lines #L378 - L380 were not covered by tests
return strings.TrimSpace(
strings.Replace(string(resBytes), "*", "", 1),
) == branch, nil
}

func (r *repo) CommitMessage(id string) (string, error) {
msgBytes, err := libExec.Exec(
r.buildCommand("log", "-n", "1", "--pretty=format:%s", id),
Expand Down
20 changes: 18 additions & 2 deletions pkg/git/git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,28 @@ func TestRepo(t *testing.T) {
require.NoError(t, err)
})

testBranch := fmt.Sprintf("test-branch-%s", uuid.NewString())
err = r.CreateChildBranch(testBranch)
require.NoError(t, err)

t.Run("can create a child branch", func(t *testing.T) {
testBranch := fmt.Sprintf("test-branch-%s", uuid.NewString())
err = r.CreateChildBranch(testBranch)
require.NoError(t, err)
})

t.Run("can check if local branch exists -- negative result", func(t *testing.T) {
var exists bool
exists, err = r.LocalBranchExists("branch-that-does-not-exist")
require.NoError(t, err)
require.False(t, exists)
})

t.Run("can check if local branch exists -- positive result", func(t *testing.T) {
var exists bool
exists, err = r.LocalBranchExists(testBranch)
require.NoError(t, err)
require.True(t, exists)
})

err = os.WriteFile(fmt.Sprintf("%s/%s", r.WorkingDir(), "test.txt"), []byte("bar"), 0600)
require.NoError(t, err)

Expand Down
Loading