Skip to content

Commit 0c98521

Browse files
nikomatsakisnikomatsakis
committed
lock issues
1 parent 99233cc commit 0c98521

File tree

2 files changed

+159
-49
lines changed

2 files changed

+159
-49
lines changed

‎mdbook-goals/src/main.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use anyhow::Context;
12
use mdbook::preprocess::{CmdPreprocessor, Preprocessor};
23
use mdbook_preprocessor::GoalPreprocessor;
34
use regex::Regex;
@@ -92,7 +93,8 @@ fn main() -> anyhow::Result<()> {
9293
commit,
9394
sleep,
9495
}) => {
95-
rfc::generate_issues(&opt.repository, path, *commit, *sleep)?;
96+
rfc::generate_issues(&opt.repository, path, *commit, *sleep)
97+
.with_context(|| format!("failed to adjust issues; rerun command to resume"))?;
9698
}
9799

98100
Some(Command::TeamRepo {

‎mdbook-goals/src/rfc.rs

Lines changed: 156 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ use crate::{
1515
team::{get_person_data, TeamName},
1616
};
1717

18+
const LOCK_TEXT: &str = "This issue is intended for status updates only.\n\nFor general questions or comments, please contact the owner(s) directly.";
19+
1820
fn validate_path(path: &Path) -> anyhow::Result<String> {
1921
if !path.is_dir() {
2022
return Err(anyhow::anyhow!(
@@ -103,50 +105,52 @@ pub fn generate_issues(
103105
commit: bool,
104106
sleep: u64,
105107
) -> anyhow::Result<()> {
106-
let timeframe = validate_path(path)?;
108+
// Hacky but works: we loop because after creating the issue, we sometimes have additional sync to do,
109+
// and it's easier this way.
110+
loop {
111+
let timeframe = validate_path(path)?;
107112

108-
let mut goal_documents = goal::goals_in_dir(path)?;
109-
goal_documents.retain(|gd| gd.is_not_not_accepted());
113+
let mut goal_documents = goal::goals_in_dir(path)?;
114+
goal_documents.retain(|gd| gd.is_not_not_accepted());
110115

111-
let teams_with_asks = teams_with_asks(&goal_documents);
112-
let mut actions = initialize_labels(repository, &teams_with_asks)?;
113-
actions.extend(initialize_issues(repository, &timeframe, &goal_documents)?);
116+
let teams_with_asks = teams_with_asks(&goal_documents);
117+
let mut actions = initialize_labels(repository, &teams_with_asks)?;
118+
actions.extend(initialize_issues(repository, &timeframe, &goal_documents)?);
114119

115-
if actions.is_empty() {
116-
eprintln!("No actions to be executed.");
117-
return Ok(());
118-
}
120+
if actions.is_empty() {
121+
return Ok(());
122+
}
119123

120-
if commit {
121-
progress_bar::init_progress_bar(actions.len());
122-
progress_bar::set_progress_bar_action(
123-
"Executing",
124-
progress_bar::Color::Blue,
125-
progress_bar::Style::Bold,
126-
);
127-
for action in actions.into_iter() {
128-
progress_bar::print_progress_bar_info(
129-
"Action",
130-
&format!("{}", action),
131-
progress_bar::Color::Green,
124+
if commit {
125+
progress_bar::init_progress_bar(actions.len());
126+
progress_bar::set_progress_bar_action(
127+
"Executing",
128+
progress_bar::Color::Blue,
132129
progress_bar::Style::Bold,
133130
);
134-
action.execute(repository, &timeframe)?;
135-
progress_bar::inc_progress_bar();
136-
137-
std::thread::sleep(Duration::from_millis(sleep));
138-
}
139-
progress_bar::finalize_progress_bar();
140-
} else {
141-
eprintln!("Actions to be executed:");
142-
for action in &actions {
143-
eprintln!("* {action}");
131+
for action in actions.into_iter() {
132+
progress_bar::print_progress_bar_info(
133+
"Action",
134+
&format!("{}", action),
135+
progress_bar::Color::Green,
136+
progress_bar::Style::Bold,
137+
);
138+
action.execute(repository, &timeframe)?;
139+
progress_bar::inc_progress_bar();
140+
141+
std::thread::sleep(Duration::from_millis(sleep));
142+
}
143+
progress_bar::finalize_progress_bar();
144+
} else {
145+
eprintln!("Actions to be executed:");
146+
for action in &actions {
147+
eprintln!("* {action}");
148+
}
149+
eprintln!("");
150+
eprintln!("Use `--commit` to execute the actions.");
151+
return Ok(());
144152
}
145-
eprintln!("");
146-
eprintln!("Use `--commit` to execute the actions.");
147153
}
148-
149-
Ok(())
150154
}
151155

152156
#[derive(Debug, PartialEq, Eq, PartialOrd, Ord)]
@@ -167,11 +171,15 @@ enum GithubAction {
167171
},
168172

169173
// We intentionally do not sync the issue *text*, because it may have been edited.
170-
SyncIssue {
174+
SyncAssignees {
171175
number: u64,
172176
remove_owners: BTreeSet<String>,
173177
add_owners: BTreeSet<String>,
174178
},
179+
180+
LockIssue {
181+
number: u64,
182+
},
175183
}
176184

177185
#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
@@ -183,14 +191,24 @@ struct GhLabel {
183191
#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
184192
struct ExistingGithubIssue {
185193
number: u64,
194+
/// Just github username, no `@`
186195
assignees: BTreeSet<String>,
196+
comments: Vec<ExistingGithubComment>,
197+
}
198+
199+
#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
200+
struct ExistingGithubComment {
201+
/// Just github username, no `@`
202+
author: String,
203+
body: String,
187204
}
188205

189206
#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
190207
struct ExistingGithubIssueJson {
191208
title: String,
192209
number: u64,
193210
assignees: Vec<ExistingGithubAssigneeJson>,
211+
comments: Vec<ExistingGithubCommentJson>,
194212
}
195213

196214
#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
@@ -199,6 +217,17 @@ struct ExistingGithubAssigneeJson {
199217
name: String,
200218
}
201219

220+
#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
221+
struct ExistingGithubCommentJson {
222+
body: String,
223+
author: ExistingGithubAuthorJson,
224+
}
225+
226+
#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
227+
struct ExistingGithubAuthorJson {
228+
login: String,
229+
}
230+
202231
fn list_labels(repository: &str) -> anyhow::Result<Vec<GhLabel>> {
203232
let output = Command::new("gh")
204233
.arg("-R")
@@ -226,7 +255,7 @@ fn list_issue_titles_in_milestone(
226255
.arg("-m")
227256
.arg(timeframe)
228257
.arg("--json")
229-
.arg("title,assignees,number")
258+
.arg("title,assignees,number,comments")
230259
.output()?;
231260

232261
let existing_issues: Vec<ExistingGithubIssueJson> = serde_json::from_slice(&output.stdout)?;
@@ -238,10 +267,14 @@ fn list_issue_titles_in_milestone(
238267
e_i.title,
239268
ExistingGithubIssue {
240269
number: e_i.number,
241-
assignees: e_i
242-
.assignees
243-
.iter()
244-
.map(|a| format!("@{}", a.login))
270+
assignees: e_i.assignees.into_iter().map(|a| a.login).collect(),
271+
comments: e_i
272+
.comments
273+
.into_iter()
274+
.map(|c| ExistingGithubComment {
275+
author: format!("@{}", c.author.login),
276+
body: c.body,
277+
})
245278
.collect(),
246279
},
247280
)
@@ -308,7 +341,7 @@ fn initialize_issues(
308341
match existing_issues.get(&desired_issue.title) {
309342
Some(existing_issue) => {
310343
if existing_issue.assignees != desired_issue.assignees {
311-
actions.insert(GithubAction::SyncIssue {
344+
actions.insert(GithubAction::SyncAssignees {
312345
number: existing_issue.number,
313346
remove_owners: existing_issue
314347
.assignees
@@ -322,6 +355,12 @@ fn initialize_issues(
322355
.collect(),
323356
});
324357
}
358+
359+
if !existing_issue.was_locked() {
360+
actions.insert(GithubAction::LockIssue {
361+
number: existing_issue.number,
362+
});
363+
}
325364
}
326365

327366
None => {
@@ -335,11 +374,19 @@ fn initialize_issues(
335374
Ok(actions)
336375
}
337376

377+
impl ExistingGithubIssue {
378+
/// We use the presence of a "lock comment" as a signal that we successfully locked the issue.
379+
/// The github CLI doesn't let you query that directly.
380+
fn was_locked(&self) -> bool {
381+
self.comments.iter().any(|c| c.body.trim() == LOCK_TEXT)
382+
}
383+
}
384+
338385
fn issue(timeframe: &str, document: &GoalDocument) -> anyhow::Result<GithubIssue> {
339386
let mut assignees = BTreeSet::default();
340387
for username in document.metadata.owner_usernames() {
341-
if get_person_data(username)?.is_some() {
342-
assignees.insert(username[1..].to_string());
388+
if let Some(data) = get_person_data(username)? {
389+
assignees.insert(data.github_username.clone());
343390
}
344391
}
345392

@@ -452,8 +499,25 @@ impl Display for GithubAction {
452499
GithubAction::CreateIssue { issue } => {
453500
write!(f, "create issue \"{}\"", issue.title)
454501
}
455-
GithubAction::SyncIssue { number, .. } => {
456-
write!(f, "sync issue #{}", number)
502+
GithubAction::SyncAssignees {
503+
number,
504+
remove_owners,
505+
add_owners,
506+
} => {
507+
write!(
508+
f,
509+
"sync issue #{} ({})",
510+
number,
511+
remove_owners
512+
.iter()
513+
.map(|s| format!("-{}", s))
514+
.chain(add_owners.iter().map(|s| format!("+{}", s)))
515+
.collect::<Vec<_>>()
516+
.join(", ")
517+
)
518+
}
519+
GithubAction::LockIssue { number } => {
520+
write!(f, "lock issue #{}", number)
457521
}
458522
}
459523
}
@@ -522,8 +586,10 @@ impl GithubAction {
522586
} else {
523587
Ok(())
524588
}
589+
590+
// Note: the issue is not locked, but we will reloop around later.
525591
}
526-
GithubAction::SyncIssue {
592+
GithubAction::SyncAssignees {
527593
number,
528594
remove_owners,
529595
add_owners,
@@ -555,8 +621,50 @@ impl GithubAction {
555621
Ok(())
556622
}
557623
}
624+
GithubAction::LockIssue { number } => lock_issue(repository, number),
625+
}
626+
}
627+
}
628+
629+
fn lock_issue(repository: &str, number: u64) -> anyhow::Result<()> {
630+
let output = Command::new("gh")
631+
.arg("-R")
632+
.arg(repository)
633+
.arg("issue")
634+
.arg("lock")
635+
.arg(number.to_string())
636+
.output()?;
637+
638+
if !output.status.success() {
639+
if !output.stderr.starts_with(b"already locked") {
640+
return Err(anyhow::anyhow!(
641+
"failed to lock issue `{}`: {}",
642+
number,
643+
String::from_utf8_lossy(&output.stderr)
644+
));
558645
}
559646
}
647+
648+
// Leave a comment explaining what is going on.
649+
let output = Command::new("gh")
650+
.arg("-R")
651+
.arg(repository)
652+
.arg("issue")
653+
.arg("comment")
654+
.arg(number.to_string())
655+
.arg("-b")
656+
.arg(LOCK_TEXT)
657+
.output()?;
658+
659+
if !output.status.success() {
660+
return Err(anyhow::anyhow!(
661+
"failed to leave lock comment `{}`: {}",
662+
number,
663+
String::from_utf8_lossy(&output.stderr)
664+
));
665+
}
666+
667+
Ok(())
560668
}
561669

562670
/// Returns a comma-separated list of the strings in `s` (no spaces).

0 commit comments

Comments
 (0)