Skip to content
This repository has been archived by the owner on May 8, 2024. It is now read-only.

Fall back to checking out HEAD #250

Merged
merged 2 commits into from
Sep 25, 2022
Merged

Fall back to checking out HEAD #250

merged 2 commits into from
Sep 25, 2022

Conversation

fkorotkov
Copy link
Contributor

In case merge ref is not available. These edge cases were cauhgt by integration tests in Cirrus CI:

  1. Cloning a PR with conflicts.
  2. Cloning a closed PR.

Related to #247 and #249

In case merge ref is not available. These edge cases were cauhgt by integration tests in Cirrus CI:

1. Cloning a PR with conflicts.
2. Cloning a closed PR.

Related to #247 and #249
@fkorotkov fkorotkov requested a review from edigaryev September 24, 2022 12:05
@fkorotkov
Copy link
Contributor Author

Tested locally by building a v1.89.0 version and running Cirrus CI's integration tests locally.

logUploader.Write([]byte("\nFailed to fetch merge ref! PR might not be mergable. Falling back to head ref..."))
headRefSpec := fmt.Sprintf("+refs/pull/%s/head:refs/remotes/origin/pull/%[1]s", pr_number)
fetchOptions.RefSpecs = []config.RefSpec{config.RefSpec(headRefSpec)}
err = repo.Fetch(fetchOptions)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we use FetchContext() here too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied the logic from below retry logic. 🤔 Will check if we need to change it in both places.

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 in 61b473b

@fkorotkov fkorotkov enabled auto-merge (squash) September 25, 2022 15:21
@fkorotkov fkorotkov merged commit 7db20cf into master Sep 25, 2022
@fkorotkov fkorotkov deleted the improved-pr-login branch September 25, 2022 15:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants