-
Notifications
You must be signed in to change notification settings - Fork 4
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
restoring checkout of update branch to be a new branch #41
base: master
Are you sure you want to change the base?
Conversation
previously we had just git checkout -b <branch> and this worked because the branch was not fetched on the github actions host - adding the OR to checkout an existing branch seems to have triggered an error. All I am doing here is restoring those lines in both recipes to what they used to be. Signed-off-by: vsoch <[email protected]>
@Beanow I think we also need a pull from master since we start with it and it wasn’t up to date on the other branch, let me know your thoughts! |
@Beanow I'm not sure that we need this anymore - the contributors graphic ran okay last night, no issues! If you agree I can close this here. |
The answer isn't immediately obvious to me here, so I agree to close it until it proves to be an actual problem :] |
@Beanow here is what I was talking about https://github.com/sourcecred/sourcecred-action/runs/520948999?check_suite_focus=true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it seems to me like there's a more fundamental issue here.
We're first generating new data, like contributors/prototype.
Then we're trying to check out the latest remote changes. Regardless of what the exact git command looks like here, I would expect there to be a failure because the local file we just generated would be overwritten by the remote branch we're trying to check out.
This isn't an issue if we don't care about the history and want to force-push a commit. But if we want to preserve history, you'll probably need either: checkout first, then generate new files, then commit-push.
Or generate, stash changes, checkout, pop stash (hope there's no conflicts), commit-push.
@@ -34,13 +34,13 @@ jobs: | |||
git remote set-url origin "https://x-access-token:${GITHUB_TOKEN}@github.com/${GITHUB_REPOSITORY}.git" | |||
git branch | |||
printf "Branch to push to is ${UPDATE_BRANCH}\n" | |||
git checkout -b ${UPDATE_BRANCH} || git checkout ${UPDATE_BRANCH} | |||
git checkout -b "${UPDATE_BRANCH}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I notice this is the only place with quotes for the branch. With quotes seems like the right way to go about it, let's apply that to other instances (at least for changed lines in this PR).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I paused working on this because we’d replace with cred-action anyway.
previously we had just git checkout -b and this worked because the branch was not fetched on the github actions host - adding the OR to checkout an existing branch seems to have triggered an error. All I am doing here is restoring those lines in both recipes to what they used to be. It's a little confusing looking at the Actions tab because although there was a failure yesterday, the contributors graphic was updated, so perhaps this was the day before? Here is the previously working version of one of the files https://github.com/sourcecred/sourcecred-action/blob/b9e2bdb4fac6f0f809adf35098bdd9220aed6e06/.github/workflows/contributors_automated.yml#L25. If it triggered again, we might want to grab changes before we run the containers.
Signed-off-by: vsoch [email protected]