From 2624f18ea01a7ab23c4fa6d12d47a57b811c9300 Mon Sep 17 00:00:00 2001 From: Ilya Grigoriev Date: Fri, 22 Dec 2023 17:39:21 -0800 Subject: [PATCH] lib: mild refactoring made possible by previous commit Inline `create_descendant_commits`, move some functionality of `DescendantRebaser::rebase_next` to `rebase_all`, a seemingly more logical location. --- lib/src/repo.rs | 22 ++++++---------------- lib/src/rewrite.rs | 14 ++++++-------- 2 files changed, 12 insertions(+), 24 deletions(-) diff --git a/lib/src/repo.rs b/lib/src/repo.rs index b1f09022d8..a6a0aba055 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -882,21 +882,6 @@ impl MutableRepo { !(self.rewritten_commits.is_empty() && self.abandoned_commits.is_empty()) } - /// Creates a `DescendantRebaser` to rebase descendants of the recorded - /// rewritten and abandoned commits. - // TODO(ilyagr): Inline this. It's only used in tests. - fn create_descendant_rebaser<'settings, 'repo>( - &'repo mut self, - settings: &'settings UserSettings, - ) -> DescendantRebaser<'settings, 'repo> { - DescendantRebaser::new( - settings, - self, - self.rewritten_commits.clone(), - self.abandoned_commits.clone(), - ) - } - /// After the rebaser returned by this function is dropped, /// self.clear_descendant_rebaser_plans() needs to be called. fn rebase_descendants_return_rebaser<'settings, 'repo>( @@ -908,7 +893,12 @@ impl MutableRepo { // Optimization return Ok(None); } - let mut rebaser = self.create_descendant_rebaser(settings); + let mut rebaser = DescendantRebaser::new( + settings, + self, + self.rewritten_commits.clone(), + self.abandoned_commits.clone(), + ); *rebaser.mut_options() = options; rebaser.rebase_all()?; Ok(Some(rebaser)) diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs index d2bccc89be..64c728470f 100644 --- a/lib/src/rewrite.rs +++ b/lib/src/rewrite.rs @@ -587,9 +587,12 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> { new_commit, })); } - // TODO: As the TODO above says, we should probably change the API. Even if we - // don't, we should at least make this code not do any work if you call - // rebase_next() after we've returned None. + Ok(None) + } + + pub fn rebase_all(&mut self) -> Result<(), TreeMergeError> { + while self.rebase_next()?.is_some() {} + // TODO: As the TODO above says, we should probably change the API. let mut view = self.mut_repo.view().store_view().clone(); for commit_id in &self.heads_to_remove { view.head_ids.remove(commit_id); @@ -600,11 +603,6 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> { self.heads_to_remove.clear(); self.heads_to_add.clear(); self.mut_repo.set_view(view); - Ok(None) - } - - pub fn rebase_all(&mut self) -> Result<(), TreeMergeError> { - while self.rebase_next()?.is_some() {} Ok(()) } }