From 32f6f598f0c2752159b2ddf007486b1d6071458f Mon Sep 17 00:00:00 2001 From: Evan Mesterhazy Date: Sun, 21 Apr 2024 08:40:44 -0400 Subject: [PATCH] CommitRewriter::rewrite_parents(): Take `IntoIterator` instead of `&[CommitId]` This makes the API more flexible for cases where the caller doesn't already have a Vec or array of owned CommitIds. CommitIds are often manipulated by reference, so this makes the API more useable in many practical cases. In many cases `rewrite_parents()` does not even need to clone the input CommitIds. This refactor allows the clone to be avoided if it's unnecessary. There might be other APIs that would benefit from a similar change. In general, it seems like there are a lot of places where we're writing `&[commit_x.id().clone, commit_y.id().clone()]` and similiar. - [Rust API Guidelines](https://rust-lang.github.io/api-guidelines/flexibility.html#functions-minimize-assumptions-about-parameters-by-using-generics-c-generic) --- lib/src/rewrite.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs index c66c8dc015..6915cb5bd5 100644 --- a/lib/src/rewrite.rs +++ b/lib/src/rewrite.rs @@ -153,10 +153,14 @@ impl<'repo> CommitRewriter<'repo> { /// Update the intended new parents by replacing any occurrence of /// `old_parent` by `new_parents`. - pub fn replace_parent(&mut self, old_parent: &CommitId, new_parents: &[CommitId]) { + pub fn replace_parent<'a>( + &mut self, + old_parent: &CommitId, + new_parents: impl IntoIterator, + ) { if let Some(i) = self.new_parents.iter().position(|p| p == old_parent) { self.new_parents - .splice(i..i + 1, new_parents.iter().cloned()); + .splice(i..i + 1, new_parents.into_iter().cloned()); let mut unique = HashSet::new(); self.new_parents.retain(|p| unique.insert(p.clone())); }