From 11f486b49934091cf656e35405e2b41ca437b27e Mon Sep 17 00:00:00 2001 From: jerryq <1397200108@qq.com> Date: Sat, 2 Aug 2025 01:21:42 +0800 Subject: [PATCH 1/3] Make `Comment::new` private and migrate existing usage --- docs/development.md | 2 +- src/bors/command/mod.rs | 2 +- src/bors/comment.rs | 210 ++++++++++++++++++++++++++++++++- src/bors/handlers/help.rs | 70 +---------- src/bors/handlers/info.rs | 59 +-------- src/bors/handlers/mod.rs | 49 ++------ src/bors/handlers/ping.rs | 6 +- src/bors/handlers/pr_events.rs | 23 +--- src/bors/handlers/review.rs | 27 ++--- 9 files changed, 239 insertions(+), 209 deletions(-) diff --git a/docs/development.md b/docs/development.md index b4a7b00a..885664d2 100644 --- a/docs/development.md +++ b/docs/development.md @@ -148,7 +148,7 @@ After that, you should commit the changes to the `.sqlx` directory. When modifying commands, make sure to update both: 1. The help page in `templates/help.html` -2. The `@bors help` command output in `src/bors/handlers/help.rs` +2. The `@bors help` command output in `src/bors/comment.rs` ## Logs in tests By default, logs are disabled in tests. To enable them, add `#[traced_test]` diff --git a/src/bors/command/mod.rs b/src/bors/command/mod.rs index 8c9b0a36..9a4ed979 100644 --- a/src/bors/command/mod.rs +++ b/src/bors/command/mod.rs @@ -88,7 +88,7 @@ impl FromStr for RollupMode { /// /// When modifying commands, remember to also update: /// - `templates/help.html` (HTML help page) -/// - `src/bors/handlers/help.rs` (the `@bors help` command output) +/// - [`crate::bors::comment::help_comment`] (the `@bors help` command output) #[derive(Debug, PartialEq)] pub enum BorsCommand { /// Approve a commit. diff --git a/src/bors/comment.rs b/src/bors/comment.rs index caab2aa8..c3f8c246 100644 --- a/src/bors/comment.rs +++ b/src/bors/comment.rs @@ -1,11 +1,14 @@ use itertools::Itertools; use octocrab::models::workflows::Job; use serde::Serialize; +use std::sync::Arc; use std::time::Duration; use crate::bors::FailedWorkflowRun; -use crate::bors::command::CommandPrefix; +use crate::bors::command::{BorsCommand, CommandParseError, CommandPrefix}; +use crate::database::{ApprovalStatus, MergeableState, PgDbClient, PullRequestModel}; use crate::github::GithubRepoName; +use crate::permissions::PermissionType; use crate::utils::text::pluralize; use crate::{ database::{WorkflowModel, WorkflowStatus}, @@ -31,7 +34,7 @@ pub enum CommentTag { } impl Comment { - pub fn new(text: String) -> Self { + fn new(text: String) -> Self { Self { text, metadata: None, @@ -51,6 +54,209 @@ impl Comment { } } +/// Format the bors command help in Markdown format. +pub fn help_comment() -> Comment { + // The help is generated manually to have a nicer structure. + // We do a no-op destructuring of `BorsCommand` to make it harder to modify help in case new + // commands are added though. + match BorsCommand::Ping { + BorsCommand::Approve { + approver: _, + rollup: _, + priority: _, + } => {} + BorsCommand::Unapprove => {} + BorsCommand::Help => {} + BorsCommand::Ping => {} + BorsCommand::Try { parent: _, jobs: _ } => {} + BorsCommand::TryCancel => {} + BorsCommand::SetPriority(_) => {} + BorsCommand::Info => {} + BorsCommand::SetDelegate(_) => {} + BorsCommand::Undelegate => {} + BorsCommand::SetRollupMode(_) => {} + BorsCommand::OpenTree => {} + BorsCommand::TreeClosed(_) => {} + } + + Comment::new(r#" +You can use the following commands: + +## PR management +- `r+ [p=] [rollup=]`: Approve this PR on your behalf + - Optionally, you can specify the `` of the PR and if it is eligible for rollups (`)`. +- `r= [p=] [rollup=]`: Approve this PR on behalf of `` + - Optionally, you can specify the `` of the PR and if it is eligible for rollups (`)`. + - You can pass a comma-separated list of GitHub usernames. +- `r-`: Unapprove this PR +- `p=` or `priority=`: Set the priority of this PR +- `rollup=`: Set the rollup status of the PR +- `rollup`: Short for `rollup=always` +- `rollup-`: Short for `rollup=maybe` +- `delegate=`: Delegate permissions for running try builds or approving to the PR author + - `try` allows the PR author to start try builds. + - `review` allows the PR author to both start try builds and approve the PR. +- `delegate+`: Delegate approval permissions to the PR author + - Shortcut for `delegate=review` +- `delegate-`: Remove any previously granted permission delegation +- `try [parent=] [jobs=]`: Start a try build. + - Optionally, you can specify a `` SHA with which will the PR be merged. You can specify `parent=last` to use the same parent SHA as the previous try build. + - Optionally, you can select a comma-separated list of CI `` to run in the try build. +- `try cancel`: Cancel a running try build +- `info`: Get information about the current PR + +## Repository management +- `treeclosed=`: Close the tree for PRs with priority less than `` +- `treeclosed-` or `treeopen`: Open the repository tree for merging + +## Meta commands +- `ping`: Check if the bot is alive +- `help`: Print this help message +"#.to_string()) +} + +pub async fn info_comment(pr: &PullRequestModel, db: Arc) -> Comment { + use std::fmt::Write; + + let mut message = format!("## Status of PR `{}`\n", pr.number); + + // Approval info + if let ApprovalStatus::Approved(info) = &pr.approval_status { + writeln!(message, "- Approved by: `{}`", info.approver).unwrap(); + } else { + writeln!(message, "- Not Approved").unwrap(); + } + + // Priority info + if let Some(priority) = pr.priority { + writeln!(message, "- Priority: {priority}").unwrap(); + } else { + writeln!(message, "- Priority: unset").unwrap(); + } + + // Mergeability state + writeln!( + message, + "- Mergeable: {}", + match pr.mergeable_state { + MergeableState::Mergeable => "yes", + MergeableState::HasConflicts => "no", + MergeableState::Unknown => "unknown", + } + ).unwrap(); + + // Try build status + if let Some(try_build) = &pr.try_build { + writeln!(message, "- Try build is in progress").unwrap(); + + if let Ok(urls) = db.get_workflow_urls_for_build(try_build).await { + message.extend( + urls.into_iter() + .map(|url| format!("\t- Workflow URL: {url}")), + ); + } + } + + // Auto build status + if let Some(auto_build) = &pr.auto_build { + writeln!(message, "- Auto build is in progress").unwrap(); + + if let Ok(urls) = db.get_workflow_urls_for_build(auto_build).await { + message.extend( + urls.into_iter() + .map(|url| format!("\t- Workflow URL: {url}")), + ); + } + } + Comment::new(message) +} + +pub fn exec_command_failed_comment() -> Comment { + Comment::new(":x: Encountered an error while executing command".to_string()) +} + +pub fn parse_command_failed_comment( + error: &CommandParseError, + bot_prefix: &CommandPrefix, +) -> Comment { + use std::fmt::Write; + + let mut message = match error { + CommandParseError::MissingCommand => "Missing command.".to_string(), + CommandParseError::UnknownCommand(command) => { + format!(r#"Unknown command "{command}"."#) + } + CommandParseError::MissingArgValue { arg } => { + format!(r#"Unknown value for argument "{arg}"."#) + } + CommandParseError::UnknownArg(arg) => { + format!(r#"Unknown argument "{arg}"."#) + } + CommandParseError::DuplicateArg(arg) => { + format!(r#"Argument "{arg}" found multiple times."#) + } + CommandParseError::ValidationError(error) => { + format!("Invalid command: {error}.") + } + }; + writeln!( + message, + " Run `{} help` to see available commands.", + bot_prefix + ) + .unwrap(); + Comment::new(message) +} + +pub fn insufficient_privileges_comment(username: &str, permission_type: PermissionType) -> Comment { + Comment::new(format!( + "@{}: :key: Insufficient privileges: not in {} users", + username, permission_type + )) +} + +pub fn pong_message() -> Comment { + Comment::new("Pong 🏓!".to_string()) +} + +pub fn edited_pr_comment(base_name: &str) -> Comment { + Comment::new(format!( + r#":warning: The base branch changed to `{base_name}`, and the +PR will need to be re-approved."#, + )) +} + +pub fn pushed_pr_comment(head_sha: &CommitSha, cancel_message: Option) -> Comment { + let mut comment = format!( + r#":warning: A new commit `{}` was pushed to the branch, the +PR will need to be re-approved."#, + head_sha + ); + if let Some(message) = cancel_message { + comment.push_str(&format!("\n\n{message}")); + } + Comment::new(comment) +} + +pub fn tree_closed_comment(priority: u32) -> Comment { + Comment::new(format!( + "Tree closed for PRs with priority less than {}", + priority + )) +} + +pub fn tree_open_comment() -> Comment { + Comment::new("Tree is now open for merging".to_string()) +} + +pub fn unapproval_comment(head_sha: &CommitSha, cancel_message: Option) -> Comment { + let mut comment = format!("Commit {} has been unapproved.", head_sha); + if let Some(message) = cancel_message { + comment.push_str(&format!("\n\n{message}")); + } + Comment::new(comment) +} + pub fn try_build_succeeded_comment( workflows: &[WorkflowModel], commit_sha: CommitSha, diff --git a/src/bors/handlers/help.rs b/src/bors/handlers/help.rs index 21beb625..44063c49 100644 --- a/src/bors/handlers/help.rs +++ b/src/bors/handlers/help.rs @@ -1,6 +1,5 @@ -use crate::bors::Comment; use crate::bors::RepositoryState; -use crate::bors::command::BorsCommand; +use crate::bors::comment::help_comment; use crate::github::PullRequestNumber; use std::sync::Arc; @@ -8,75 +7,10 @@ pub(super) async fn command_help( repo: Arc, pr_number: PullRequestNumber, ) -> anyhow::Result<()> { - let help = format_help(); - - repo.client - .post_comment(pr_number, Comment::new(help.to_string())) - .await?; + repo.client.post_comment(pr_number, help_comment()).await?; Ok(()) } -/// Format the bors command help in Markdown format. -fn format_help() -> &'static str { - // The help is generated manually to have a nicer structure. - // We do a no-op destructuring of `BorsCommand` to make it harder to modify help in case new - // commands are added though. - match BorsCommand::Ping { - BorsCommand::Approve { - approver: _, - rollup: _, - priority: _, - } => {} - BorsCommand::Unapprove => {} - BorsCommand::Help => {} - BorsCommand::Ping => {} - BorsCommand::Try { parent: _, jobs: _ } => {} - BorsCommand::TryCancel => {} - BorsCommand::SetPriority(_) => {} - BorsCommand::Info => {} - BorsCommand::SetDelegate(_) => {} - BorsCommand::Undelegate => {} - BorsCommand::SetRollupMode(_) => {} - BorsCommand::OpenTree => {} - BorsCommand::TreeClosed(_) => {} - } - - r#" -You can use the following commands: - -## PR management -- `r+ [p=] [rollup=]`: Approve this PR on your behalf - - Optionally, you can specify the `` of the PR and if it is eligible for rollups (`)`. -- `r= [p=] [rollup=]`: Approve this PR on behalf of `` - - Optionally, you can specify the `` of the PR and if it is eligible for rollups (`)`. - - You can pass a comma-separated list of GitHub usernames. -- `r-`: Unapprove this PR -- `p=` or `priority=`: Set the priority of this PR -- `rollup=`: Set the rollup status of the PR -- `rollup`: Short for `rollup=always` -- `rollup-`: Short for `rollup=maybe` -- `delegate=`: Delegate permissions for running try builds or approving to the PR author - - `try` allows the PR author to start try builds. - - `review` allows the PR author to both start try builds and approve the PR. -- `delegate+`: Delegate approval permissions to the PR author - - Shortcut for `delegate=review` -- `delegate-`: Remove any previously granted permission delegation -- `try [parent=] [jobs=]`: Start a try build. - - Optionally, you can specify a `` SHA with which will the PR be merged. You can specify `parent=last` to use the same parent SHA as the previous try build. - - Optionally, you can select a comma-separated list of CI `` to run in the try build. -- `try cancel`: Cancel a running try build -- `info`: Get information about the current PR - -## Repository management -- `treeclosed=`: Close the tree for PRs with priority less than `` -- `treeclosed-` or `treeopen`: Open the repository tree for merging - -## Meta commands -- `ping`: Check if the bot is alive -- `help`: Print this help message -"# -} - #[cfg(test)] mod tests { use crate::tests::{BorsTester, run_test}; diff --git a/src/bors/handlers/info.rs b/src/bors/handlers/info.rs index d1147f3d..d949aa2f 100644 --- a/src/bors/handlers/info.rs +++ b/src/bors/handlers/info.rs @@ -1,8 +1,7 @@ -use crate::bors::Comment; use crate::bors::RepositoryState; +use crate::bors::comment::info_comment; use crate::bors::handlers::PullRequestData; use crate::database::PgDbClient; -use crate::database::{ApprovalStatus, MergeableState}; use std::sync::Arc; pub(super) async fn command_info( @@ -10,63 +9,9 @@ pub(super) async fn command_info( pr: &PullRequestData, db: Arc, ) -> anyhow::Result<()> { - use std::fmt::Write; - - let mut message = format!("## Status of PR `{}`\n", pr.number()); - - // Approval info - if let ApprovalStatus::Approved(info) = &pr.db.approval_status { - writeln!(message, "- Approved by: `{}`", info.approver)?; - } else { - writeln!(message, "- Not Approved")?; - } - - // Priority info - if let Some(priority) = pr.db.priority { - writeln!(message, "- Priority: {priority}")?; - } else { - writeln!(message, "- Priority: unset")?; - } - - // Mergeability state - writeln!( - message, - "- Mergeable: {}", - match pr.db.mergeable_state { - MergeableState::Mergeable => "yes", - MergeableState::HasConflicts => "no", - MergeableState::Unknown => "unknown", - } - )?; - - // Try build status - if let Some(try_build) = &pr.db.try_build { - writeln!(message, "- Try build is in progress")?; - - if let Ok(urls) = db.get_workflow_urls_for_build(try_build).await { - message.extend( - urls.into_iter() - .map(|url| format!("\t- Workflow URL: {url}")), - ); - } - } - - // Auto build status - if let Some(auto_build) = &pr.db.auto_build { - writeln!(message, "- Auto build is in progress")?; - - if let Ok(urls) = db.get_workflow_urls_for_build(auto_build).await { - message.extend( - urls.into_iter() - .map(|url| format!("\t- Workflow URL: {url}")), - ); - } - } - repo.client - .post_comment(pr.number(), Comment::new(message)) + .post_comment(pr.number(), info_comment(&pr.db, db).await) .await?; - Ok(()) } diff --git a/src/bors/handlers/mod.rs b/src/bors/handlers/mod.rs index 73031e7b..dbc98ecf 100644 --- a/src/bors/handlers/mod.rs +++ b/src/bors/handlers/mod.rs @@ -2,6 +2,9 @@ use std::sync::Arc; use super::mergeable_queue::MergeableQueueSender; use crate::bors::command::{BorsCommand, CommandParseError}; +use crate::bors::comment::{ + exec_command_failed_comment, insufficient_privileges_comment, parse_command_failed_comment, +}; use crate::bors::event::{BorsGlobalEvent, BorsRepositoryEvent, PullRequestComment}; use crate::bors::handlers::help::command_help; use crate::bors::handlers::info::command_info; @@ -20,7 +23,7 @@ use crate::bors::handlers::review::{ use crate::bors::handlers::trybuild::{TRY_BRANCH_NAME, command_try_build, command_try_cancel}; use crate::bors::handlers::workflow::{handle_workflow_completed, handle_workflow_started}; use crate::bors::merge_queue::{AUTO_BRANCH_NAME, MergeQueueSender}; -use crate::bors::{BorsContext, CommandPrefix, Comment, RepositoryState}; +use crate::bors::{BorsContext, CommandPrefix, RepositoryState}; use crate::database::{DelegatedPermission, PullRequestModel}; use crate::github::{GithubUser, LabelTrigger, PullRequest, PullRequestNumber}; use crate::permissions::PermissionType; @@ -87,12 +90,7 @@ pub async fn handle_bors_repository_event( .await { repo.client - .post_comment( - pr_number, - Comment::new( - ":x: Encountered an error while executing command".to_string(), - ), - ) + .post_comment(pr_number, exec_command_failed_comment()) .await .context("Cannot send comment reacting to an error")?; return Err(error.context("Cannot perform command")); @@ -334,8 +332,6 @@ async fn handle_comment( comment: PullRequestComment, merge_queue_tx: MergeQueueSender, ) -> anyhow::Result<()> { - use std::fmt::Write; - let pr_number = comment.pr_number; let mut commands = ctx.parser.parse_commands(&comment.text); @@ -505,32 +501,12 @@ async fn handle_comment( } } Err(error) => { - let mut message = match error { - CommandParseError::MissingCommand => "Missing command.".to_string(), - CommandParseError::UnknownCommand(command) => { - format!(r#"Unknown command "{command}"."#) - } - CommandParseError::MissingArgValue { arg } => { - format!(r#"Unknown value for argument "{arg}"."#) - } - CommandParseError::UnknownArg(arg) => { - format!(r#"Unknown argument "{arg}"."#) - } - CommandParseError::DuplicateArg(arg) => { - format!(r#"Argument "{arg}" found multiple times."#) - } - CommandParseError::ValidationError(error) => { - format!("Invalid command: {error}.") - } - }; - writeln!( - message, - " Run `{} help` to see available commands.", - ctx.parser.prefix() - )?; - tracing::warn!("{}", message); + tracing::warn!("{:?}", error); repo.client - .post_comment(pr_github.number, Comment::new(message)) + .post_comment( + pr_github.number, + parse_command_failed_comment(&error, ctx.parser.prefix()), + ) .await .context("Could not reply to PR comment")?; } @@ -605,10 +581,7 @@ async fn deny_request( repo.client .post_comment( pr_number, - Comment::new(format!( - "@{}: :key: Insufficient privileges: not in {} users", - author.username, permission_type - )), + insufficient_privileges_comment(&author.username, permission_type), ) .await?; Ok(()) diff --git a/src/bors/handlers/ping.rs b/src/bors/handlers/ping.rs index 06a6a6ff..e922b04f 100644 --- a/src/bors/handlers/ping.rs +++ b/src/bors/handlers/ping.rs @@ -1,16 +1,14 @@ use std::sync::Arc; -use crate::bors::Comment; use crate::bors::RepositoryState; +use crate::bors::comment::pong_message; use crate::github::PullRequestNumber; pub(super) async fn command_ping( repo: Arc, pr_number: PullRequestNumber, ) -> anyhow::Result<()> { - repo.client - .post_comment(pr_number, Comment::new("Pong 🏓!".to_string())) - .await?; + repo.client.post_comment(pr_number, pong_message()).await?; Ok(()) } diff --git a/src/bors/handlers/pr_events.rs b/src/bors/handlers/pr_events.rs index 75c4a3ec..0f6f7832 100644 --- a/src/bors/handlers/pr_events.rs +++ b/src/bors/handlers/pr_events.rs @@ -1,4 +1,5 @@ use crate::PgDbClient; +use crate::bors::comment::{edited_pr_comment, pushed_pr_comment}; use crate::bors::event::{ PullRequestAssigned, PullRequestClosed, PullRequestConvertedToDraft, PullRequestEdited, PullRequestMerged, PullRequestOpened, PullRequestPushed, PullRequestReadyForReview, @@ -7,7 +8,7 @@ use crate::bors::event::{ use crate::bors::handlers::unapprove_pr; use crate::bors::handlers::workflow::{AutoBuildCancelReason, maybe_cancel_auto_build}; use crate::bors::mergeable_queue::MergeableQueueSender; -use crate::bors::{Comment, PullRequestStatus, RepositoryState}; +use crate::bors::{PullRequestStatus, RepositoryState}; use crate::database::MergeableState; use crate::github::{CommitSha, PullRequestNumber}; use crate::utils::text::pluralize; @@ -251,13 +252,7 @@ async fn notify_of_edited_pr( base_name: &str, ) -> anyhow::Result<()> { repo.client - .post_comment( - pr_number, - Comment::new(format!( - r#":warning: The base branch changed to `{base_name}`, and the -PR will need to be re-approved."#, - )), - ) + .post_comment(pr_number, edited_pr_comment(base_name)) .await?; Ok(()) } @@ -268,18 +263,8 @@ async fn notify_of_pushed_pr( head_sha: CommitSha, cancel_message: Option, ) -> anyhow::Result<()> { - let mut comment = format!( - r#":warning: A new commit `{}` was pushed to the branch, the -PR will need to be re-approved."#, - head_sha - ); - - if let Some(message) = cancel_message { - comment.push_str(&format!("\n\n{message}")); - } - repo.client - .post_comment(pr_number, Comment::new(comment)) + .post_comment(pr_number, pushed_pr_comment(&head_sha, cancel_message)) .await?; Ok(()) } diff --git a/src/bors/handlers/review.rs b/src/bors/handlers/review.rs index 8989f9e9..75b3676a 100644 --- a/src/bors/handlers/review.rs +++ b/src/bors/handlers/review.rs @@ -5,7 +5,8 @@ use crate::bors::command::RollupMode; use crate::bors::command::{Approver, CommandPrefix}; use crate::bors::comment::{ approve_blocking_labels_present, approve_non_open_pr_comment, approve_wip_title, - approved_comment, delegate_comment, delegate_try_builds_comment, unapprove_non_open_pr_comment, + approved_comment, delegate_comment, delegate_try_builds_comment, tree_closed_comment, + tree_open_comment, unapproval_comment, unapprove_non_open_pr_comment, }; use crate::bors::handlers::labels::handle_label_trigger; use crate::bors::handlers::workflow::{AutoBuildCancelReason, maybe_cancel_auto_build}; @@ -289,13 +290,7 @@ async fn notify_of_tree_closed( priority: u32, ) -> anyhow::Result<()> { repo.client - .post_comment( - pr_number, - Comment::new(format!( - "Tree closed for PRs with priority less than {}", - priority - )), - ) + .post_comment(pr_number, tree_closed_comment(priority)) .await?; Ok(()) } @@ -305,10 +300,7 @@ async fn notify_of_tree_open( pr_number: PullRequestNumber, ) -> anyhow::Result<()> { repo.client - .post_comment( - pr_number, - Comment::new("Tree is now open for merging".to_string()), - ) + .post_comment(pr_number, tree_open_comment()) .await?; Ok(()) } @@ -318,14 +310,11 @@ async fn notify_of_unapproval( pr: &PullRequestData, cancel_message: Option, ) -> anyhow::Result<()> { - let mut comment = format!("Commit {} has been unapproved.", pr.github.head.sha); - - if let Some(message) = cancel_message { - comment.push_str(&format!("\n\n{message}")); - } - repo.client - .post_comment(pr.number(), Comment::new(comment)) + .post_comment( + pr.number(), + unapproval_comment(&pr.github.head.sha, cancel_message), + ) .await?; Ok(()) } From 13dfefdb07a2c0f5398109aeb84874201a94c82d Mon Sep 17 00:00:00 2001 From: jerryq <1397200108@qq.com> Date: Sat, 2 Aug 2025 01:26:53 +0800 Subject: [PATCH 2/3] Delete multiple `std::fmt::Write` and make `Comment::new` accept `impl Into` --- src/bors/comment.rs | 43 +++++++++++++++++++------------------------ 1 file changed, 19 insertions(+), 24 deletions(-) diff --git a/src/bors/comment.rs b/src/bors/comment.rs index c3f8c246..3cc05292 100644 --- a/src/bors/comment.rs +++ b/src/bors/comment.rs @@ -1,6 +1,7 @@ use itertools::Itertools; use octocrab::models::workflows::Job; use serde::Serialize; +use std::fmt::Write; use std::sync::Arc; use std::time::Duration; @@ -34,9 +35,9 @@ pub enum CommentTag { } impl Comment { - fn new(text: String) -> Self { + fn new(text: impl Into) -> Self { Self { - text, + text: text.into(), metadata: None, } } @@ -79,7 +80,8 @@ pub fn help_comment() -> Comment { BorsCommand::TreeClosed(_) => {} } - Comment::new(r#" + Comment::new( + r#" You can use the following commands: ## PR management @@ -112,12 +114,11 @@ You can use the following commands: ## Meta commands - `ping`: Check if the bot is alive - `help`: Print this help message -"#.to_string()) +"#, + ) } pub async fn info_comment(pr: &PullRequestModel, db: Arc) -> Comment { - use std::fmt::Write; - let mut message = format!("## Status of PR `{}`\n", pr.number); // Approval info @@ -143,7 +144,8 @@ pub async fn info_comment(pr: &PullRequestModel, db: Arc) -> Comment MergeableState::HasConflicts => "no", MergeableState::Unknown => "unknown", } - ).unwrap(); + ) + .unwrap(); // Try build status if let Some(try_build) = &pr.try_build { @@ -172,15 +174,13 @@ pub async fn info_comment(pr: &PullRequestModel, db: Arc) -> Comment } pub fn exec_command_failed_comment() -> Comment { - Comment::new(":x: Encountered an error while executing command".to_string()) + Comment::new(":x: Encountered an error while executing command") } pub fn parse_command_failed_comment( error: &CommandParseError, bot_prefix: &CommandPrefix, ) -> Comment { - use std::fmt::Write; - let mut message = match error { CommandParseError::MissingCommand => "Missing command.".to_string(), CommandParseError::UnknownCommand(command) => { @@ -216,7 +216,7 @@ pub fn insufficient_privileges_comment(username: &str, permission_type: Permissi } pub fn pong_message() -> Comment { - Comment::new("Pong 🏓!".to_string()) + Comment::new("Pong 🏓!") } pub fn edited_pr_comment(base_name: &str) -> Comment { @@ -246,7 +246,7 @@ pub fn tree_closed_comment(priority: u32) -> Comment { } pub fn tree_open_comment() -> Comment { - Comment::new("Tree is now open for merging".to_string()) + Comment::new("Tree is now open for merging") } pub fn unapproval_comment(head_sha: &CommitSha, cancel_message: Option) -> Comment { @@ -262,8 +262,6 @@ pub fn try_build_succeeded_comment( commit_sha: CommitSha, parent_sha: CommitSha, ) -> Comment { - use std::fmt::Write; - let mut text = String::from(":sunny: Try build successful"); // If there is only a single workflow (the common case), compress the output @@ -289,17 +287,17 @@ pub fn try_build_succeeded_comment( } pub fn cant_find_last_parent_comment() -> Comment { - Comment::new(":exclamation: There was no previous build. Please set an explicit parent or remove the `parent=last` argument to use the default parent.".to_string()) + Comment::new( + ":exclamation: There was no previous build. Please set an explicit parent or remove the `parent=last` argument to use the default parent.", + ) } pub fn no_try_build_in_progress_comment() -> Comment { - Comment::new(":exclamation: There is currently no try build in progress.".to_string()) + Comment::new(":exclamation: There is currently no try build in progress.") } pub fn try_build_cancelled_with_failed_workflow_cancel_comment() -> Comment { - Comment::new( - "Try build was cancelled. It was not possible to cancel some workflows.".to_string(), - ) + Comment::new("Try build was cancelled. It was not possible to cancel some workflows.") } pub fn try_build_cancelled_comment(workflow_urls: impl Iterator) -> Comment { @@ -316,8 +314,6 @@ pub fn build_failed_comment( commit_sha: CommitSha, failed_workflows: Vec, ) -> Comment { - use std::fmt::Write; - let mut msg = format!(":broken_heart: Test for {commit_sha} failed"); let mut workflow_links = failed_workflows .iter() @@ -376,7 +372,6 @@ pub fn try_build_started_comment( bot_prefix: &CommandPrefix, cancelled_workflow_urls: Vec, ) -> Comment { - use std::fmt::Write; let mut msg = format!(":hourglass: Trying commit {head_sha} with merge {merge_sha}…\n\n"); if !cancelled_workflow_urls.is_empty() { @@ -447,11 +442,11 @@ It is now in the [queue]({web_url}/queue/{}) for this repository. } pub fn approve_non_open_pr_comment() -> Comment { - Comment::new(":clipboard: Only open, non-draft PRs can be approved.".to_string()) + Comment::new(":clipboard: Only open, non-draft PRs can be approved.") } pub fn unapprove_non_open_pr_comment() -> Comment { - Comment::new(":clipboard: Only unclosed PRs can be unapproved.".to_string()) + Comment::new(":clipboard: Only unclosed PRs can be unapproved.") } pub fn approve_wip_title(keyword: &str) -> Comment { From 36d7fcc76547aeface7996e5fd9fa260e7399232 Mon Sep 17 00:00:00 2001 From: jerryq <1397200108@qq.com> Date: Sat, 2 Aug 2025 01:33:23 +0800 Subject: [PATCH 3/3] Minor improvements --- src/bors/comment.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/bors/comment.rs b/src/bors/comment.rs index 3cc05292..636decb8 100644 --- a/src/bors/comment.rs +++ b/src/bors/comment.rs @@ -301,12 +301,11 @@ pub fn try_build_cancelled_with_failed_workflow_cancel_comment() -> Comment { } pub fn try_build_cancelled_comment(workflow_urls: impl Iterator) -> Comment { - let mut try_build_cancelled_comment = - r#"Try build cancelled. Cancelled workflows:"#.to_string(); + let mut comment = String::from("Try build cancelled. Cancelled workflows:"); for url in workflow_urls { - try_build_cancelled_comment += format!("\n- {}", url).as_str(); + write!(comment, "\n- {}", url).unwrap(); } - Comment::new(try_build_cancelled_comment) + Comment::new(comment) } pub fn build_failed_comment( @@ -511,7 +510,6 @@ fn list_workflows_status(workflows: &[WorkflowModel]) -> String { } ) }) - .collect::>() .join("\n") } @@ -531,7 +529,6 @@ pub fn auto_build_succeeded_comment( let urls = workflows .iter() .map(|w| format!("[{}]({})", w.name, w.url)) - .collect::>() .join(", "); Comment::new(format!(