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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: handle branch names mismatch #1708

Merged
merged 13 commits into from Oct 1, 2018

Conversation

Projects
None yet
4 participants
@vanessayuenn
Contributor

vanessayuenn commented Sep 27, 2018

Description of the Change

This PR introduces the use of refspec in push and pull operations (fetch does it a bit differently), in order to handle situations where the local branch is named differently from its tracking branch. This set of circumstances has become less of an edge case since the introduction of PR-checkout -- we prefix the branch name with PR number, etc.

Benefits

  • Able to fetch, pull, push checked out PRs
  • Able to be creative in naming your local branches 馃懐

Possible Drawbacks

This PR changes how the underlying git operations work, which is used by many things in the package; so the possible setback is if things break in edge cases that I haven't thought of.

Applicable Issues

fixes: #1697
fixes: #918

Metrics

N/A

Tests

Added unit tests to verify that push & pull & fetch work as expected despite the difference between local branch name and its upstream branch name.

Manually tested push & pull & fetch operations in these cases:

  • local branch named differently from upstream
  • local branch named the same as upstream
  • checked out PR from a branch in the original repo
  • checked out PR opened from a forked repo
@coveralls

This comment has been minimized.

coveralls commented Sep 27, 2018

Coverage Status

Coverage increased (+0.08%) to 82.028% when pulling a1324c5 on vy/handle-branch-names-mismatch into 7950c43 on master.

@smashwilson

馃憤

Show resolved Hide resolved lib/models/branch.js
@annthurium

nice work! Thanks for taking this on.

.instance()
.pull(await wrapper.instance().fetchData(repository))();
const postPullSHA = await repository.git.exec(['rev-parse', 'HEAD']);
assert.isTrue(repository.pull.called);

This comment has been minimized.

@annthurium

annthurium Sep 27, 2018

Contributor

nit: do you want to assert that repository.pull was called with the args you expect?

.instance()
.push(await wrapper.instance().fetchData(repository))();
const remoteSHA = await repository.git.exec(['rev-parse', 'origin/master']);
assert.isTrue(repository.push.called);

This comment has been minimized.

@annthurium

annthurium Sep 27, 2018

Contributor

same nit as above - might be good to specify which args push should be called with.

vanessayuenn added some commits Sep 28, 2018

@vanessayuenn vanessayuenn merged commit 46afadd into master Oct 1, 2018

6 checks passed

ci/circleci: beta Your tests passed on CircleCI!
Details
ci/circleci: dev Your tests passed on CircleCI!
Details
ci/circleci: stable Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.08%) to 82.028%
Details

@vanessayuenn vanessayuenn deleted the vy/handle-branch-names-mismatch branch Oct 1, 2018

@annthurium annthurium referenced this pull request Oct 20, 2018

Closed

v0.21.0-0 QA Review #1752

7 of 7 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment