Skip to content

Commit b50324a

Browse files
committed
Use destructuring in diffs to make it harder to forget diff fields
1 parent e5728e1 commit b50324a

File tree

1 file changed

+93
-37
lines changed

1 file changed

+93
-37
lines changed

sync-team/src/github/mod.rs

Lines changed: 93 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -676,25 +676,33 @@ impl CreateRepoDiff {
676676

677677
impl std::fmt::Display for CreateRepoDiff {
678678
fn fmt(&self, mut f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
679+
let CreateRepoDiff {
680+
org,
681+
name,
682+
settings,
683+
permissions,
684+
branch_protections,
685+
} = self;
686+
679687
let RepoSettings {
680688
description,
681689
homepage,
682690
archived: _,
683691
auto_merge_enabled,
684-
} = &self.settings;
692+
} = &settings;
685693

686694
writeln!(f, "➕ Creating repo:")?;
687-
writeln!(f, " Org: {}", self.org)?;
688-
writeln!(f, " Name: {}", self.name)?;
689-
writeln!(f, " Description: {:?}", description)?;
690-
writeln!(f, " Homepage: {:?}", homepage)?;
691-
writeln!(f, " Auto-merge: {}", auto_merge_enabled)?;
695+
writeln!(f, " Org: {org}")?;
696+
writeln!(f, " Name: {name}")?;
697+
writeln!(f, " Description: {description}")?;
698+
writeln!(f, " Homepage: {homepage:?}")?;
699+
writeln!(f, " Auto-merge: {auto_merge_enabled}")?;
692700
writeln!(f, " Permissions:")?;
693-
for diff in &self.permissions {
701+
for diff in permissions {
694702
write!(f, "{diff}")?;
695703
}
696704
writeln!(f, " Branch Protections:")?;
697-
for (branch_name, branch_protection) in &self.branch_protections {
705+
for (branch_name, branch_protection) in branch_protections {
698706
writeln!(&mut f, " {branch_name}")?;
699707
log_branch_protection(branch_protection, None, &mut f)?;
700708
}
@@ -719,9 +727,18 @@ impl UpdateRepoDiff {
719727
return true;
720728
}
721729

722-
self.settings_diff.0 == self.settings_diff.1
723-
&& self.permission_diffs.is_empty()
724-
&& self.branch_protection_diffs.is_empty()
730+
let UpdateRepoDiff {
731+
org: _,
732+
name: _,
733+
repo_node_id: _,
734+
settings_diff,
735+
permission_diffs,
736+
branch_protection_diffs,
737+
} = self;
738+
739+
settings_diff.0 == settings_diff.1
740+
&& permission_diffs.is_empty()
741+
&& branch_protection_diffs.is_empty()
725742
}
726743

727744
fn can_be_modified(&self) -> bool {
@@ -770,8 +787,18 @@ impl std::fmt::Display for UpdateRepoDiff {
770787
if self.noop() {
771788
return Ok(());
772789
}
773-
writeln!(f, "📝 Editing repo '{}/{}':", self.org, self.name)?;
774-
let (settings_old, settings_new) = &self.settings_diff;
790+
791+
let UpdateRepoDiff {
792+
org,
793+
name,
794+
repo_node_id: _,
795+
settings_diff,
796+
permission_diffs,
797+
branch_protection_diffs,
798+
} = self;
799+
800+
writeln!(f, "📝 Editing repo '{org}/{name}':")?;
801+
let (settings_old, settings_new) = &settings_diff;
775802
let RepoSettings {
776803
description,
777804
homepage,
@@ -803,17 +830,17 @@ impl std::fmt::Display for UpdateRepoDiff {
803830
(true, false) => writeln!(f, " Disable auto-merge")?,
804831
_ => {}
805832
}
806-
if !self.permission_diffs.is_empty() {
833+
if !permission_diffs.is_empty() {
807834
writeln!(f, " Permission Changes:")?;
835+
for permission_diff in permission_diffs {
836+
write!(f, "{permission_diff}")?;
837+
}
808838
}
809-
for permission_diff in &self.permission_diffs {
810-
write!(f, "{permission_diff}")?;
811-
}
812-
if !self.branch_protection_diffs.is_empty() {
839+
if !branch_protection_diffs.is_empty() {
813840
writeln!(f, " Branch Protections:")?;
814-
}
815-
for branch_protection_diff in &self.branch_protection_diffs {
816-
write!(f, "{branch_protection_diff}")?;
841+
for branch_protection_diff in branch_protection_diffs {
842+
write!(f, "{branch_protection_diff}")?;
843+
}
817844
}
818845

819846
Ok(())
@@ -854,11 +881,13 @@ impl RepoPermissionAssignmentDiff {
854881

855882
impl std::fmt::Display for RepoPermissionAssignmentDiff {
856883
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
857-
let name = match &self.collaborator {
884+
let RepoPermissionAssignmentDiff { collaborator, diff } = self;
885+
886+
let name = match &collaborator {
858887
RepoCollaborator::Team(name) => format!("team '{name}'"),
859888
RepoCollaborator::User(name) => format!("user '{name}'"),
860889
};
861-
match &self.diff {
890+
match &diff {
862891
RepoPermissionDiff::Create(p) => {
863892
writeln!(f, " Giving {name} {p} permission")
864893
}
@@ -1056,20 +1085,28 @@ impl CreateTeamDiff {
10561085

10571086
impl std::fmt::Display for CreateTeamDiff {
10581087
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
1088+
let CreateTeamDiff {
1089+
org,
1090+
name,
1091+
description,
1092+
privacy,
1093+
members,
1094+
} = self;
1095+
10591096
writeln!(f, "➕ Creating team:")?;
1060-
writeln!(f, " Org: {}", self.org)?;
1061-
writeln!(f, " Name: {}", self.name)?;
1062-
writeln!(f, " Description: {}", self.description)?;
1097+
writeln!(f, " Org: {org}")?;
1098+
writeln!(f, " Name: {name}")?;
1099+
writeln!(f, " Description: {description}")?;
10631100
writeln!(
10641101
f,
10651102
" Privacy: {}",
1066-
match self.privacy {
1103+
match privacy {
10671104
TeamPrivacy::Secret => "secret",
10681105
TeamPrivacy::Closed => "closed",
10691106
}
10701107
)?;
10711108
writeln!(f, " Members:")?;
1072-
for (name, role) in &self.members {
1109+
for (name, role) in members {
10731110
writeln!(f, " {name}: {role}")?;
10741111
}
10751112
Ok(())
@@ -1109,10 +1146,19 @@ impl EditTeamDiff {
11091146
}
11101147

11111148
fn noop(&self) -> bool {
1112-
self.name_diff.is_none()
1113-
&& self.description_diff.is_none()
1114-
&& self.privacy_diff.is_none()
1115-
&& self.member_diffs.iter().all(|(_, d)| d.is_noop())
1149+
let EditTeamDiff {
1150+
org: _,
1151+
name: _,
1152+
name_diff,
1153+
description_diff,
1154+
privacy_diff,
1155+
member_diffs,
1156+
} = self;
1157+
1158+
name_diff.is_none()
1159+
&& description_diff.is_none()
1160+
&& privacy_diff.is_none()
1161+
&& member_diffs.iter().all(|(_, d)| d.is_noop())
11161162
}
11171163
}
11181164

@@ -1121,21 +1167,31 @@ impl std::fmt::Display for EditTeamDiff {
11211167
if self.noop() {
11221168
return Ok(());
11231169
}
1124-
writeln!(f, "📝 Editing team '{}/{}':", self.org, self.name)?;
1125-
if let Some(n) = &self.name_diff {
1170+
1171+
let EditTeamDiff {
1172+
org,
1173+
name,
1174+
name_diff,
1175+
description_diff,
1176+
privacy_diff,
1177+
member_diffs,
1178+
} = self;
1179+
1180+
writeln!(f, "📝 Editing team '{org}/{name}':")?;
1181+
if let Some(n) = name_diff {
11261182
writeln!(f, " New name: {n}")?;
11271183
}
1128-
if let Some((old, new)) = &self.description_diff {
1184+
if let Some((old, new)) = &description_diff {
11291185
writeln!(f, " New description: '{old}' => '{new}'")?;
11301186
}
1131-
if let Some((old, new)) = &self.privacy_diff {
1187+
if let Some((old, new)) = &privacy_diff {
11321188
let display = |privacy: &TeamPrivacy| match privacy {
11331189
TeamPrivacy::Secret => "secret",
11341190
TeamPrivacy::Closed => "closed",
11351191
};
11361192
writeln!(f, " New privacy: '{}' => '{}'", display(old), display(new))?;
11371193
}
1138-
for (member, diff) in &self.member_diffs {
1194+
for (member, diff) in member_diffs {
11391195
match diff {
11401196
MemberDiff::Create(r) => {
11411197
writeln!(f, " Adding member '{member}' with {r} role")?;

0 commit comments

Comments
 (0)