Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 36 additions & 22 deletions src/github.rs
Original file line number Diff line number Diff line change
Expand Up @@ -785,33 +785,47 @@ impl Issue {
Ok(())
}

pub async fn remove_label(&self, client: &GithubClient, label: &str) -> anyhow::Result<()> {
log::info!("remove_label from {}: {:?}", self.global_id(), label);
// DELETE /repos/:owner/:repo/issues/:number/labels/{name}
let url = format!(
"{repo_url}/issues/{number}/labels/{name}",
repo_url = self.repository().url(client),
number = self.number,
name = label,
pub async fn remove_labels(
&self,
client: &GithubClient,
labels: Vec<Label>,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's unclear to me if taking a Vec<Label> is the right choice, in most cases we need to convert Vec<&str>/Vec<String> to it. Maybe we should take an impl IntoIterator<Item = &str> or something like that.

Copy link
Member

@Kobzol Kobzol Oct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine, although I'd use &[Label], but that doesn't matter that much (it would probably require more generic annotations).

) -> anyhow::Result<()> {
log::info!("remove_labels from {}: {:?}", self.global_id(), labels);

// Don't try to remove labels not already present on this issue.
let labels = labels
.into_iter()
.filter(|l| self.labels().contains(l))
.collect::<Vec<_>>();

log::info!(
"remove_labels: {} filtered to {:?}",
self.global_id(),
labels
);

if !self
.labels()
.iter()
.any(|l| l.name.to_lowercase() == label.to_lowercase())
{
log::info!(
"remove_label from {}: {:?} already not present, skipping",
self.global_id(),
label
);
if labels.is_empty() {
return Ok(());
}

client
.send_req(client.delete(&url))
.await
.context("failed to delete label")?;
// There is no API to remove all labels at once, so we issue as many
// API requests are required in parallel.
let requests = labels.into_iter().map(|label| async move {
// DELETE /repos/:owner/:repo/issues/:number/labels/{name}
let url = format!(
"{repo_url}/issues/{number}/labels/{name}",
repo_url = self.repository().url(client),
number = self.number,
name = label.name,
);

client
.send_req(client.delete(&url))
.await
.with_context(|| format!("failed to remove {label:?}"))
});

futures::future::try_join_all(requests).await?;

Ok(())
}
Expand Down
19 changes: 6 additions & 13 deletions src/handlers/autolabel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,18 +222,11 @@ pub(super) async fn handle_input(
}
}

for label in input.remove {
event
.issue
.remove_label(&ctx.github, &label.name)
.await
.with_context(|| {
format!(
"failed to remove {:?} from {:?}",
label,
event.issue.global_id()
)
})?;
}
event
.issue
.remove_labels(&ctx.github, input.remove)
.await
.context("failed to remove labels from the issue")?;

Ok(())
}
18 changes: 11 additions & 7 deletions src/handlers/check_commits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,13 +243,17 @@ async fn handle_new_state(

// Remove the labels no longer required
if !labels_to_remove.is_empty() {
for label in labels_to_remove {
event
.issue
.remove_label(&ctx.github, &label)
.await
.context("failed to remove a label in check_commits")?;
}
event
.issue
.remove_labels(
&ctx.github,
labels_to_remove
.into_iter()
.map(|name| Label { name })
.collect(),
)
.await
.context("failed to remove a label in check_commits")?;
}

// Add the labels that are now required
Expand Down
19 changes: 13 additions & 6 deletions src/handlers/concern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,12 +152,19 @@ pub(super) async fn handle_command(
).await.context("unable to post the comment failure it-self")?;
}
} else {
for l in &config.labels {
issue
.remove_label(&ctx.github, &l)
.await
.context("unable to remove the concern labels")?;
}
issue
.remove_labels(
&ctx.github,
config
.labels
.iter()
.map(|l| Label {
name: l.to_string(),
})
.collect(),
)
.await
.context("unable to remove the concern labels")?;
}

// Apply the new markdown concerns list to the issue
Expand Down
7 changes: 6 additions & 1 deletion src/handlers/major_change.rs
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,12 @@ As the automated representative, I would like to thank the author for their work
.await
.context("unable to add the accept label")?;
issue
.remove_label(&ctx.github, &config.second_label)
.remove_labels(
&ctx.github,
vec![Label {
name: config.second_label.clone(),
}],
)
.await
.context("unable to remove the second label")?;
issue
Expand Down
10 changes: 7 additions & 3 deletions src/handlers/merge_conflicts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,15 +288,19 @@ async fn maybe_add_comment(

let current_labels: HashSet<_> = issue.labels.iter().map(|l| l.name.clone()).collect();
if current_labels.is_disjoint(&config.unless) {
for label in &config.remove {
issue.remove_label(gh, label).await?;
}
let to_add = config
.add
.iter()
.map(|l| Label { name: l.clone() })
.collect();
let to_remove = config
.remove
.iter()
.map(|l| Label { name: l.clone() })
.collect();

issue.add_labels(gh, to_add).await?;
issue.remove_labels(gh, to_remove).await?;
}

Ok(())
Expand Down
18 changes: 8 additions & 10 deletions src/handlers/relabel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,16 +73,14 @@ pub(super) async fn handle_command(
}

// Remove labels
for label in to_remove {
if let Err(e) = issue.remove_label(&ctx.github, &label.name).await {
tracing::error!(
"failed to remove {:?} from issue {}: {:?}",
label,
issue.global_id(),
e
);
return Err(e);
}
if let Err(e) = issue.remove_labels(&ctx.github, to_remove.clone()).await {
tracing::error!(
"failed to remove {:?} from issue {}: {:?}",
to_remove,
issue.global_id(),
e
);
return Err(e);
}

Ok(())
Expand Down
15 changes: 12 additions & 3 deletions src/handlers/review_requested.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,18 @@ pub(crate) async fn handle_input(
)
.await?;

for label in &config.remove_labels {
event.issue.remove_label(&ctx.github, label).await?;
}
event
.issue
.remove_labels(
&ctx.github,
config
.remove_labels
.iter()
.cloned()
.map(|name| Label { name })
.collect(),
)
.await?;

Ok(())
}
17 changes: 14 additions & 3 deletions src/handlers/review_submitted.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,20 @@ pub(crate) async fn handle(

if event.issue.assignees.contains(&event.comment.user) {
// Remove review labels
for label in &config.review_labels {
event.issue.remove_label(&ctx.github, &label).await?;
}
event
.issue
.remove_labels(
&ctx.github,
config
.review_labels
.iter()
.map(|label| Label {
name: label.clone(),
})
.collect(),
)
.await?;

// Add waiting on author
event
.issue
Expand Down
16 changes: 11 additions & 5 deletions src/handlers/shortcut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,17 @@ pub(super) async fn handle_command(
};

if !issue_labels.iter().any(|l| l.name == add) {
for remove in status_labels {
if remove != add {
issue.remove_label(&ctx.github, remove).await?;
}
}
issue
.remove_labels(
&ctx.github,
status_labels
.iter()
.filter(|l| **l != add)
.map(|l| Label { name: (*l).into() })
.collect(),
)
.await?;

issue
.add_labels(
&ctx.github,
Expand Down
Loading