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

Checkout after creating a new branch #384

Conversation

katainaka0503
Copy link
Contributor

@katainaka0503 katainaka0503 commented Feb 20, 2022

I'm a big fan of the new feature below. It will allow us more flexible GitOps operations.

#304

However, I found a bug after trying the feature by myself on my local environment.

It seems after creating a new branch, Argo CD doesn't checkout the newly created branch. In the end, the new branch is pushed without any changes committed.

So, I've moved checkout process to after branch creation.

@CLAassistant
Copy link

CLAassistant commented Feb 20, 2022

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@jannfis jannfis left a comment

Choose a reason for hiding this comment

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

Thanks @katainaka0503! I just have a question, please see below.

@nick-homex since you initially contributed the write-back branch feature, can you please have a look at this as well?

@@ -1786,7 +1786,7 @@ func Test_CommitUpdates(t *testing.T) {
defer cleanup()
gitMock.On("Checkout", mock.Anything).Run(func(args mock.Arguments) {
args.Assert(t, "mydefaultbranch")
}).Return(nil)
}).Return(nil).Once()
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please explain why the .Once() is necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your comment.

This is because in the current code, after checking out the source branch, argo cd checks out push branch .

But, technically, this is not necessary.
I'll fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed it!

@katainaka0503 katainaka0503 force-pushed the fix-write-checkout-branch-separation-feature branch from e712be2 to 41ba108 Compare February 21, 2022 16:59
Copy link
Contributor

@jannfis jannfis left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you, @katainaka0503 !

@jannfis jannfis merged commit 474a576 into argoproj-labs:master Feb 23, 2022
@katainaka0503 katainaka0503 deleted the fix-write-checkout-branch-separation-feature branch February 23, 2022 08:57
sribiere-jellysmack pushed a commit to sribiere-jellysmack/argocd-image-updater that referenced this pull request Aug 13, 2024
* Checkout after creating a new branch

* Fix test

* Fix unnecessary checkout
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants