Repository navigation
Allow comparing Branch objects - #1234
Conversation
There was a problem hiding this comment.
Haven't yet checked by myself. Does libgit2 have or plan to have an compare functions for branch or references?
There was a problem hiding this comment.
It allows comparing references with git_reference_cmp() which is what impl<'repo> Ord for Reference<'repo> uses
| let mut b1 = repo.branch("foo", &commit, false).unwrap(); | ||
| assert!(!b1.is_head()); | ||
| repo.branch("foo2", &commit, false).unwrap(); | ||
| let b2 = repo.branch("foo2", &commit, false).unwrap(); |
There was a problem hiding this comment.
Have we considered the equality here be based on branch name instead of Reference equality? It is a bit weird and too implicit when two different branch names differ comparsion says there are the same.
Perhaps we should be more explicit on the comparsion, and don't implement the eq operator?
There was a problem hiding this comment.
What if you had two different references (lowercase) to the same branch, but the underlying References are not the same - I would be surprised if those two branches are considered equal
There was a problem hiding this comment.
Agree with that one, too.
Semantically a reference is composed of target and name and oid. However upstream libgit2 has a bug that didn't consider them when comparing. They fixed that in libgit2/libgit2#6346 but won't release it until libgit2 v2.
I think until then we might not want to tie to one about-to-change semantic. What do you think?
(#851 has done a good research btw)
There was a problem hiding this comment.
No objections to waiting, but do we know when libgit2 v2 will come out?
There was a problem hiding this comment.
I have no idea to be honest. See
|
@rustbot label +S-blocked +upstream wait for upstream change in libgit2 v2 probably |
rustbot
added
S-blocked
This comment has been minimized.
This comment has been minimized.
|
Could you rebase it? I am tempted to merge it and test rc1 |
|
@rustbot author |
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
Add implementations for `PartialOrd`, `Ord`, `PartialEq`, and `Eq`. The actual is delegated to the underlying `Reference` object. Closes rust-lang#422
4a68e32 to
c16e761
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
@rustbot ready |
rustbot
added
S-waiting-on-review
3 participants
Add implementations for
PartialOrd,Ord,PartialEq, andEq. The actual is delegated to the underlyingReferenceobject.Closes #422