support non-origin remotes #44
Conversation
src/core.js
Outdated
| @@ -104,8 +104,12 @@ async function sync(cloneUrl, localDirectory, {ref, checkout}) { | |||
| } | |||
|
|
|||
| await git.cwd(localDirectory); | |||
|
|
|||
| await git.removeRemote(cloneName).catch((error) => {}); // eslint-disable-line no-unused-vars | |||
There was a problem hiding this comment.
this cloneUrl is specific to the github-token so if a token is revoked, then all the old remotes with invalid tokens are still there and will fail on the fetch.
always doing a .removeRemote() right here means the cloneUrl provided is always used.
| log.info(`> Fetching ${ref}...`); | ||
| await git.fetch('origin', ref); | ||
| const remoteName = cloneName || 'origin'; | ||
| await git.removeRemote(remoteName).catch((error) => {}); // eslint-disable-line no-unused-vars |
There was a problem hiding this comment.
this cloneUrl is specific to the github-token so if a token is revoked, then all the old remotes with invalid tokens are still there and will fail on the fetch.
always doing a .removeRemote() right here means the cloneUrl provided is always used.
removeRemote caught because we don't care if it's removing something that isn't there.
| await git.fetch('origin', ref); | ||
| const remoteName = cloneName || 'origin'; | ||
| await git.removeRemote(remoteName).catch((error) => {}); // eslint-disable-line no-unused-vars | ||
| await git.addRemote(remoteName, cloneUrl); |
There was a problem hiding this comment.
fwiw these strings are like "username/reponame", and git is totally cool with slashes in remote names, afaict.
There was a problem hiding this comment.
We also shouldn't have to validate because GitHub users shouldn't be able to create a branch with invalid characters to begin with! Should fail further upstream, not at the stage-ci level 🚀
There was a problem hiding this comment.
@paulirish This looks perfect!
From looking at the GitLab docs, it looks like the equivalent of pull_request.head.repo.full_name is pull_request.project.path_with_namespace but I'll have a play around in my own branch 👍
fixes #38