Feat/traversal sort by committer date #272
Conversation
oknozor
force-pushed
the
feat/traversal-sort-by-committer-date
branch
2 times, most recently
from
51302dd to
e9ff883
Compare
git-object/src/commit/ref_iter.rs
Outdated
| /// TODO: | ||
| pub fn committer(&mut self) -> Option<git_actor::SignatureRef<'_>> { | ||
| self.filter_map(Result::ok) | ||
| .find(|t| matches!(t, Token::Committer { .. })) |
There was a problem hiding this comment.
With find_map(…) one could reduce complexity.
git-traverse/src/commit.rs
Outdated
| fn next_by_commit_date(&mut self) -> Option<Result<ObjectId, Error>> { | ||
| let state = self.state.borrow_mut(); | ||
| let res = state.next.pop_front(); | ||
| let mut parents_with_date = vec![]; |
There was a problem hiding this comment.
This array should be part of the state to avoid re-allocation each time one calls next().
With that in place, one would probably clear it here.
oknozor
force-pushed
the
feat/traversal-sort-by-committer-date
branch
3 times, most recently
from
b45302a to
8187883
Compare
|
I have updated according to your review, let me know if I still need to change anything. |
git-traverse/src/commit.rs
Outdated
| for token in commit_iter { | ||
| match token { | ||
| Ok(git_object::commit::ref_iter::Token::Parent { id }) => { | ||
| let mut vec = vec![]; |
There was a problem hiding this comment.
Sorry I missed this the first time around. Since another buffer is needed, it should probably be part of the state as well.
git-traverse/src/commit.rs
Outdated
|
|
||
| state | ||
| .parents_with_date | ||
| .sort_by(|(_, time), (_, other_time)| other_time.cmp(&time)); |
There was a problem hiding this comment.
Here I would find time.cmp(&other_time).reverse() clearer, the reverse() makes a bit more obvious that this is going to be descending, as opposed to comparing the arguments in another order.
oknozor
force-pushed
the
feat/traversal-sort-by-committer-date
branch
from
8187883 to
2ab315f
Compare
|
Just updated according to the latest review |
oknozor
force-pushed
the
feat/traversal-sort-by-committer-date
branch
from
2ab315f to
eb36a3d
Compare
|
Thanks a lot for contributing! I hope thus far the experience was enjoyable, and if so, I would be happy to help you find another task. (And if not, I'd be eager to learn so the experience can be improved, of course). |
|
Definitely an enjoyable experience :). I'd be eager to work on something else ! |
|
Awesome, thank you! Candidates for a similar amount of work, give or take, would be #251 or #11. If these aren't for you, you could give me a hint of what interests you, or peruse the collaboration board yourself for clues. |
This is a work in progress for #270