Skip to content

Conversation

@dvtkrlbs
Copy link
Contributor

Handles the missing Date based reflog lookup. The target date is compared against the signature time field. This is my first contribution and was kinda a blind one so open to some guidance (especially on testing this addition)

@dvtkrlbs
Copy link
Contributor Author

also just realized my min_by logic is wrong since it only deal with values smaller than the target date I think

@dvtkrlbs dvtkrlbs marked this pull request as draft October 24, 2024 03:14
@dvtkrlbs
Copy link
Contributor Author

Hmm I see there is an implementation for to_time in gix_date however this is not public should I just implement Sub for Time where the Output is i64?

@dvtkrlbs dvtkrlbs force-pushed the refloglookup-date branch 5 times, most recently from dafe686 to 0187a55 Compare October 24, 2024 04:04
Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Thanks a lot, this improvement - this probably means that the Planned variant can be removed as well, nothing else uses it.

Further, please have one commit per crate, with gix-date getting its own feat: … commit.

Thanks again.

return None;
}
},
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

question since this is duplicated with the other part should I create a closure. This duplication is probably fine but it kinda bothers me.

Copy link
Member

Choose a reason for hiding this comment

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

Refactor at will, but please do so in a separate commit.

@dvtkrlbs dvtkrlbs force-pushed the refloglookup-date branch 2 times, most recently from e118f9f to 6b26435 Compare October 24, 2024 20:02
@Byron
Copy link
Member

Byron commented Nov 25, 2024

Is this ready for review?

@dvtkrlbs
Copy link
Contributor Author

hey sorry was not able to test it properly since I am not actually knowledgable about the internals of git that much. I would appreaciate if someone would test it since they would do it better than me.

@Byron Byron self-assigned this Nov 29, 2024
@Byron
Copy link
Member

Byron commented Nov 29, 2024

Thanks for letting me know - I will take care of it, hopefully so this PR is merged before next year.

@Byron Byron force-pushed the refloglookup-date branch from 6b26435 to 3f5c117 Compare December 21, 2024 14:22
@Byron Byron marked this pull request as ready for review December 21, 2024 14:22
@Byron Byron enabled auto-merge December 21, 2024 14:23
@Byron Byron force-pushed the refloglookup-date branch from 3f5c117 to 1bb5f7d Compare December 21, 2024 14:36
…be read.

Otherwise, the revlog iteration may stop unexpectedly on long lines.
- deduplicate implementation
- fix implementation to work just like Git does (according to their source code)
- add tests
@Byron Byron force-pushed the refloglookup-date branch from 1bb5f7d to 9662bc1 Compare December 21, 2024 14:44
@Byron Byron merged commit cbdbb8a into GitoxideLabs:main Dec 21, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants