Skip to content

support non-origin remotes#44

Open
paulirish wants to merge 5 commits into
zpnk:masterfrom
paulirish:nonoriginremotes
Open

support non-origin remotes#44
paulirish wants to merge 5 commits into
zpnk:masterfrom
paulirish:nonoriginremotes

Conversation

@paulirish

@paulirish paulirish commented May 11, 2018

Copy link
Copy Markdown
Contributor

fixes #38

Comment thread src/core.js Outdated

await git.cwd(localDirectory);

await git.removeRemote(cloneName).catch((error) => {}); // eslint-disable-line no-unused-vars

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

Comment thread src/core.js
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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

Comment thread src/core.js
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);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fwiw these strings are like "username/reponame", and git is totally cool with slashes in remote names, afaict.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 🚀

@hugomd hugomd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@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 👍

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.

Cannot find remote ref <branchname>

2 participants