Skip to content
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

Make check_rewritable take an iterator of &CommitId instead of &Commit #3437

Merged
merged 1 commit into from
Apr 4, 2024

Conversation

emesterhazy
Copy link
Collaborator

This function doesn't actually need commits, it only needs their IDs. In some contexts we may only have commit IDs, so there's no need to require an iterator of Commits.

This commit also adds a CommitIteratorExt that makes it easy to convert an iterator of &Commit to an iterator of &CommitId.

@jonathantanmy
Copy link
Collaborator

Looks good to me. check_rewritable does not need to revwalk the given commits at all (the revwalk is from the repository's information about which commits are immutable) so it makes sense that the minimum information needed is only the ID. Also, RevsetIteratorExt is already a thing so CommitIteratorExt is fine.

Copy link
Collaborator

@yuja yuja left a comment

Choose a reason for hiding this comment

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

I personally wouldn't add extension trait (because it's relatively easy to write .map(|c| c.id())), but I'm not against adding it either.

cli/src/commands/new.rs Outdated Show resolved Hide resolved
cli/src/commands/unsquash.rs Outdated Show resolved Hide resolved
lib/src/commit.rs Outdated Show resolved Hide resolved
@emesterhazy emesterhazy force-pushed the push-xmkqtumzqspt branch 2 times, most recently from fa59510 to 8de7a17 Compare April 4, 2024 01:51
This function doesn't actually need commits, it only needs their IDs. In some
contexts we may only have commit IDs, so there's no need to require an iterator
of Commits.

This commit also adds a `CommitIteratorExt` that makes it easy to convert an
iterator of `&Commit` to an iterator of `&CommitId`.
@emesterhazy emesterhazy merged commit d4a0477 into main Apr 4, 2024
16 checks passed
@emesterhazy emesterhazy deleted the push-xmkqtumzqspt branch April 4, 2024 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants