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

fix: use API for creating branch and fetching hashes when committing using Github API #2070

Merged
merged 4 commits into from
Apr 29, 2024

Conversation

MattiasAng
Copy link
Contributor

@MattiasAng MattiasAng commented Apr 29, 2024

Related to #1914 and issues described in this comment.

Note: I will probably not have time to add any tests this week, so feel free to take over @olblak or if approve if you feel it's good enough since most are API actions that can't be properly tested without integration tests.

The previous implementation used native Git to both create the working branch and fetching latest commit hash used when committing using API. This caused issues when using multiple targets with the same SCM due to the local cloned repository not having pulled the latest changes from the remote branch, which meant that PushBranch reverted changes done in the first target that ran.

To fix this, I've added the following:

  • Latest commit hashes are fetched from Github API instead of the native Git handler.
  • Branch is now created using the Github API if an existing branch is not found.

Test

Additional Information

Potential improvement

The queryHeadOid function use much of the same code as queryRepository but this function fails if the working branch is not available due to it being used as an input. We could probably rework this to be a more dynamic struct, but I think having an explicit function that only fetches the required information (i.e. hashes) makes sense even if it's not following DRY.

@olblak
Copy link
Member

olblak commented Apr 29, 2024

Thanks for the quick pullrequest, as you said, integration testing is not easy in that area of the code.
I'll run some manual test but since this feature is still in "experimental" mode, I'll probably rush the test

@olblak olblak added the bug Something isn't working label Apr 29, 2024
@olblak
Copy link
Member

olblak commented Apr 29, 2024

I am going to merge as I need to do some testing in that area anyway because of this issue
#2068

@olblak
Copy link
Member

olblak commented Apr 29, 2024

Thanks for the quick pullrequest

@olblak olblak merged commit 4ea55d9 into updatecli:main Apr 29, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants