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

Use git show-ref instead of git branch for better future-proofness. #967

Merged
merged 3 commits into from May 30, 2013

Conversation

Projects
None yet
4 participants
@carljm
Contributor

carljm commented May 29, 2013

Based on original patch by Anrs Hu in #966.

Passes all tests; I think it reproduces the previous behavior but more reliably.

@carljm carljm referenced this pull request May 29, 2013

Closed

bugfix for Git >=1.8.3 #966

@carljm

This comment has been minimized.

Show comment
Hide comment
@carljm

carljm May 29, 2013

Contributor

Actually I think this change could go a bit further and clean things up even a bit more; show-ref returns tags as well, so we could eliminate the distinction between get_branch_revs and get_tag_revs entirely (since they are always used together anyway) and just have a single get_refs method that returns a dictionary including both branch and tag refs.

Contributor

carljm commented May 29, 2013

Actually I think this change could go a bit further and clean things up even a bit more; show-ref returns tags as well, so we could eliminate the distinction between get_branch_revs and get_tag_revs entirely (since they are always used together anyway) and just have a single get_refs method that returns a dictionary including both branch and tag refs.

carljm added some commits May 29, 2013

Merge branch 'develop' into bugfix-git183-v2
* develop:
  add back in old tox.ini
  add back test lost in the restructure
  more functional/unit tests restructure
  break up tests dir into data, unit tests, and functional tests
@carljm

This comment has been minimized.

Show comment
Hide comment
@carljm

carljm May 29, 2013

Contributor

Ok, I think this is ready for review and merge. It just combines the previous get_tag_revs and get_branch_revs method into a single get_refs method using git show-ref, which should be less likely to change in the future since it's intended for machine consumption.

As a side effect, this also fixes some code in get_src_requirement that hadn't been working at all (because it was trying to look up commit IDs in dictionaries keyed by branch/tag names).

Contributor

carljm commented May 29, 2013

Ok, I think this is ready for review and merge. It just combines the previous get_tag_revs and get_branch_revs method into a single get_refs method using git show-ref, which should be less likely to change in the future since it's intended for machine consumption.

As a side effect, this also fixes some code in get_src_requirement that hadn't been working at all (because it was trying to look up commit IDs in dictionaries keyed by branch/tag names).

@anrs

This comment has been minimized.

Show comment
Hide comment
@anrs

anrs commented May 30, 2013

LGTM.

jezdez added a commit that referenced this pull request May 30, 2013

Merge pull request #967 from carljm/bugfix-git183-v2
Use git show-ref instead of git branch for better future-proofness.

@jezdez jezdez merged commit b2874f5 into pypa:develop May 30, 2013

1 check passed

default The Travis CI build passed
Details
@cjerdonek

This comment has been minimized.

Show comment
Hide comment
@cjerdonek

cjerdonek Jan 8, 2014

Member

It looks like this change may have caused issue #1083.

Member

cjerdonek commented on fa81a41 Jan 8, 2014

It looks like this change may have caused issue #1083.

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