Skip to content

Commit 1ba8832

Browse files
Address review feedback: simplify changes.rs code
- Change match back to if let at lines 125 and 175 (cleaner for single-data-case patterns) - Simplify iterator chaining at line 57 (use vec with extend instead of iter::once + chain) Co-authored-by: Christian Schilling <christian-schilling@users.noreply.github.com>
1 parent 3e47fc9 commit 1ba8832

File tree

1 file changed

+35
-46
lines changed

1 file changed

+35
-46
lines changed

josh-core/src/changes.rs

Lines changed: 35 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,8 @@ fn split_changes(
5454
.map(|(_, commit, _)| repo.find_commit(*commit))
5555
.collect::<Result<Vec<_>, _>>()?;
5656

57-
let base_tree = std::iter::once(repo.find_commit(base)?.tree()?);
58-
let trees: Vec<git2::Tree> = base_tree
59-
.chain(
60-
commits
61-
.iter()
62-
.map(|commit| commit.tree())
63-
.collect::<Result<Vec<_>, _>>()?,
64-
)
65-
.collect();
57+
let mut trees = vec![repo.find_commit(base)?.tree()?];
58+
trees.extend(commits.iter().map(|commit| commit.tree()).collect::<Result<Vec<_>, _>>()?);
6659

6760
let diffs: Vec<git2::Diff> = trees
6861
.windows(2)
@@ -122,24 +115,21 @@ pub fn changes_to_refs(
122115

123116
let mut seen = std::collections::HashSet::new();
124117
for change in changes.iter() {
125-
match &change.id {
126-
Some(id) => {
127-
if id.contains('@') {
128-
return Err(josh_error("Change id must not contain '@'"));
129-
}
130-
if !seen.insert(id) {
131-
return Err(josh_error(&format!(
132-
"rejecting to push {:?} with duplicate label",
133-
change.commit
134-
)));
135-
}
118+
if let Some(id) = &change.id {
119+
if id.contains('@') {
120+
return Err(josh_error("Change id must not contain '@'"));
136121
}
137-
None => {
122+
if !seen.insert(id) {
138123
return Err(josh_error(&format!(
139-
"rejecting to push {:?} without id",
124+
"rejecting to push {:?} with duplicate label",
140125
change.commit
141126
)));
142127
}
128+
} else {
129+
return Err(josh_error(&format!(
130+
"rejecting to push {:?} without id",
131+
change.commit
132+
)));
143133
}
144134
}
145135

@@ -172,38 +162,37 @@ pub fn build_to_push(
172162
oid_to_push: git2::Oid,
173163
base_oid: git2::Oid,
174164
) -> JoshResult<Vec<(String, git2::Oid, String)>> {
175-
match changes {
176-
Some(changes) => {
177-
let mut push_refs = changes_to_refs(baseref, author, changes)?;
178-
179-
if push_mode == PushMode::Split {
180-
split_changes(repo, &mut push_refs, base_oid)?;
181-
}
165+
if let Some(changes) = changes {
166+
let mut push_refs = changes_to_refs(baseref, author, changes)?;
182167

183-
if push_mode == PushMode::Review {
184-
push_refs.push((
185-
ref_with_options.to_string(),
186-
oid_to_push,
187-
"JOSH_PUSH".to_string(),
188-
));
189-
}
168+
if push_mode == PushMode::Split {
169+
split_changes(repo, &mut push_refs, base_oid)?;
170+
}
190171

172+
if push_mode == PushMode::Review {
191173
push_refs.push((
192-
format!(
193-
"refs/heads/@heads/{}/{}",
194-
baseref.replacen("refs/heads/", "", 1),
195-
author,
196-
),
174+
ref_with_options.to_string(),
197175
oid_to_push,
198-
baseref.replacen("refs/heads/", "", 1),
176+
"JOSH_PUSH".to_string(),
199177
));
200-
201-
Ok(push_refs)
202178
}
203-
None => Ok(vec![(
179+
180+
push_refs.push((
181+
format!(
182+
"refs/heads/@heads/{}/{}",
183+
baseref.replacen("refs/heads/", "", 1),
184+
author,
185+
),
186+
oid_to_push,
187+
baseref.replacen("refs/heads/", "", 1),
188+
));
189+
190+
Ok(push_refs)
191+
} else {
192+
Ok(vec![(
204193
ref_with_options.to_string(),
205194
oid_to_push,
206195
"JOSH_PUSH".to_string(),
207-
)]),
196+
)])
208197
}
209198
}

0 commit comments

Comments
 (0)