diff --git a/src/bin/bors.rs b/src/bin/bors.rs index 65de9de5..7a6b1585 100644 --- a/src/bin/bors.rs +++ b/src/bin/bors.rs @@ -137,7 +137,7 @@ fn try_main(opts: Opts) -> anyhow::Result<()> { let db = Arc::new(db); let ctx = BorsContext::new( - CommandParser::new(opts.cmd_prefix.clone()), + CommandParser::new(opts.cmd_prefix.clone().into()), db.clone(), repos.clone(), ); @@ -194,7 +194,7 @@ fn try_main(opts: Opts) -> anyhow::Result<()> { WebhookSecret::new(opts.webhook_secret), repos, db, - opts.cmd_prefix, + opts.cmd_prefix.into(), ); let server_process = webhook_server(state); diff --git a/src/bors/command/mod.rs b/src/bors/command/mod.rs index 12e29574..8c9b0a36 100644 --- a/src/bors/command/mod.rs +++ b/src/bors/command/mod.rs @@ -1,5 +1,6 @@ mod parser; use std::fmt; +use std::fmt::{Display, Formatter}; use std::str::FromStr; use crate::{database::DelegatedPermission, github::CommitSha}; @@ -8,6 +9,27 @@ pub use parser::{CommandParseError, CommandParser}; /// Priority of a commit. pub type Priority = u32; +/// Command prefix used to identify bors commands, e.g. `@bors`. +pub struct CommandPrefix(String); + +impl From for CommandPrefix { + fn from(value: String) -> Self { + Self(value) + } +} + +impl Display for CommandPrefix { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + self.0.fmt(f) + } +} + +impl AsRef for CommandPrefix { + fn as_ref(&self) -> &str { + self.0.as_str() + } +} + /// Type of parent allowed in a try build #[derive(Clone, Debug, PartialEq)] pub enum Parent { diff --git a/src/bors/command/parser.rs b/src/bors/command/parser.rs index 7a1f9455..ed213fd5 100644 --- a/src/bors/command/parser.rs +++ b/src/bors/command/parser.rs @@ -1,6 +1,6 @@ //! Defines parsers for bors commands. -use crate::bors::command::{Approver, BorsCommand, Parent}; +use crate::bors::command::{Approver, BorsCommand, CommandPrefix, Parent}; use crate::database::DelegatedPermission; use crate::github::CommitSha; use pulldown_cmark::{Event, Parser, Tag, TagEnd, TextMergeStream}; @@ -27,17 +27,17 @@ enum CommandPart<'a> { } pub struct CommandParser { - prefix: String, + prefix: CommandPrefix, } impl CommandParser { - pub fn new(prefix: String) -> Self { + pub fn new(prefix: CommandPrefix) -> Self { Self { prefix } } /// Prefix of the bot, used to invoke commands from PR comments. /// For example `@bors`. - pub fn prefix(&self) -> &str { + pub fn prefix(&self) -> &CommandPrefix { &self.prefix } @@ -49,9 +49,9 @@ impl CommandParser { let segments = extract_text_from_markdown(text); segments .lines() - .filter_map(|line| match line.find(&self.prefix) { + .filter_map(|line| match line.find(self.prefix.as_ref()) { Some(index) => { - let input = &line[index + self.prefix.len()..]; + let input = &line[index + self.prefix.as_ref().len()..]; parse_command(input) } None => None, @@ -1346,6 +1346,6 @@ I am markdown HTML comment } fn parse_commands(text: &str) -> Vec> { - CommandParser::new("@bors".to_string()).parse_commands(text) + CommandParser::new("@bors".to_string().into()).parse_commands(text) } } diff --git a/src/bors/comment.rs b/src/bors/comment.rs index ca606cb1..5c456251 100644 --- a/src/bors/comment.rs +++ b/src/bors/comment.rs @@ -1,5 +1,6 @@ use serde::Serialize; +use crate::bors::command::CommandPrefix; use crate::database::BuildModel; use crate::{ database::{WorkflowModel, WorkflowStatus}, @@ -105,7 +106,7 @@ pub fn workflow_failed_comment(workflows: &[WorkflowModel]) -> Comment { pub fn try_build_started_comment( head_sha: &CommitSha, merge_sha: &CommitSha, - bot_prefix: &str, + bot_prefix: &CommandPrefix, cancelled_workflow_urls: Vec, ) -> Comment { use std::fmt::Write; @@ -167,6 +168,26 @@ pub fn approve_non_open_pr_comment() -> Comment { Comment::new("Only open, non-draft PRs can be approved".to_string()) } +pub fn delegate_try_builds_comment(delegatee: &str, bot_prefix: &CommandPrefix) -> Comment { + Comment::new(format!( + r":v: @{delegatee}, you can now perform try builds on this pull request! + +You can now post `{bot_prefix} try` to start a try build. + ", + )) +} + +/// `delegatee` is the user who received the delegation privileges, while `delegator` is the user +/// who gave these privileges to the `delegatee`. +pub fn delegate_comment(delegatee: &str, delegator: &str, bot_prefix: &CommandPrefix) -> Comment { + Comment::new(format!( + r#":v: @{delegatee}, you can now approve this pull request! + +If @{delegator} told you to "`r=me`" after making some further change, then please make that change and post `{bot_prefix} r={delegator}`. +"# + )) +} + fn list_workflows_status(workflows: &[WorkflowModel]) -> String { workflows .iter() diff --git a/src/bors/handlers/mod.rs b/src/bors/handlers/mod.rs index 9f45a643..80ef4e82 100644 --- a/src/bors/handlers/mod.rs +++ b/src/bors/handlers/mod.rs @@ -446,9 +446,16 @@ async fn handle_comment( } BorsCommand::SetDelegate(delegate_type) => { let span = tracing::info_span!("Delegate"); - command_delegate(repo, database, &pr, &comment.author, delegate_type) - .instrument(span) - .await + command_delegate( + repo, + database, + &pr, + &comment.author, + delegate_type, + ctx.parser.prefix(), + ) + .instrument(span) + .await } BorsCommand::Undelegate => { let span = tracing::info_span!("Undelegate"); diff --git a/src/bors/handlers/review.rs b/src/bors/handlers/review.rs index 5ceb2b24..ac5161da 100644 --- a/src/bors/handlers/review.rs +++ b/src/bors/handlers/review.rs @@ -2,9 +2,11 @@ use std::sync::Arc; use crate::PgDbClient; use crate::bors::RepositoryState; -use crate::bors::command::Approver; use crate::bors::command::RollupMode; -use crate::bors::comment::approve_non_open_pr_comment; +use crate::bors::command::{Approver, CommandPrefix}; +use crate::bors::comment::{ + approve_non_open_pr_comment, delegate_comment, delegate_try_builds_comment, +}; use crate::bors::handlers::has_permission; use crate::bors::handlers::labels::handle_label_trigger; use crate::bors::handlers::{PullRequestData, deny_request}; @@ -102,6 +104,7 @@ pub(super) async fn command_delegate( pr: &PullRequestData, author: &GithubUser, delegated_permission: DelegatedPermission, + bot_prefix: &CommandPrefix, ) -> anyhow::Result<()> { tracing::info!( "Delegating PR {} {} permissions", @@ -118,7 +121,9 @@ pub(super) async fn command_delegate( &repo_state, pr.number(), &pr.github.author.username, + &author.username, delegated_permission, + bot_prefix, ) .await } @@ -268,19 +273,16 @@ async fn notify_of_delegation( repo: &RepositoryState, pr_number: PullRequestNumber, delegatee: &str, + delegator: &str, delegated_permission: DelegatedPermission, + bot_prefix: &CommandPrefix, ) -> anyhow::Result<()> { - let message = match delegated_permission { - DelegatedPermission::Try => format!( - "@{} can now perform try builds on this pull request", - delegatee - ), - DelegatedPermission::Review => format!("@{} can now approve this pull request", delegatee), + let comment = match delegated_permission { + DelegatedPermission::Try => delegate_try_builds_comment(delegatee, bot_prefix), + DelegatedPermission::Review => delegate_comment(delegatee, delegator, bot_prefix), }; - repo.client - .post_comment(pr_number, Comment::new(message)) - .await + repo.client.post_comment(pr_number, comment).await } #[cfg(test)] @@ -542,7 +544,11 @@ mod tests { .await?; insta::assert_snapshot!( tester.get_comment().await?, - @"@default-user can now approve this pull request" + @r#" + :v: @default-user, you can now approve this pull request! + + If @reviewer told you to "`r=me`" after making some further change, then please make that change and post `@bors r=reviewer`. + "# ); tester @@ -980,7 +986,11 @@ mod tests { .await?; insta::assert_snapshot!( tester.get_comment().await?, - @"@default-user can now perform try builds on this pull request" + @r" + :v: @default-user, you can now perform try builds on this pull request! + + You can now post `@bors try` to start a try build. + " ); tester .default_pr() diff --git a/src/bors/handlers/trybuild.rs b/src/bors/handlers/trybuild.rs index db881138..0071b3dc 100644 --- a/src/bors/handlers/trybuild.rs +++ b/src/bors/handlers/trybuild.rs @@ -2,7 +2,7 @@ use std::sync::Arc; use crate::PgDbClient; use crate::bors::RepositoryState; -use crate::bors::command::Parent; +use crate::bors::command::{CommandPrefix, Parent}; use crate::bors::comment::no_try_build_in_progress_comment; use crate::bors::comment::try_build_cancelled_comment; use crate::bors::comment::unclean_try_build_cancelled_comment; @@ -51,7 +51,7 @@ pub(super) async fn command_try_build( author: &GithubUser, parent: Option, jobs: Vec, - bot_prefix: &str, + bot_prefix: &CommandPrefix, ) -> anyhow::Result<()> { let repo = repo.as_ref(); if !has_permission(repo, author, pr, PermissionType::Try).await? { diff --git a/src/bors/mod.rs b/src/bors/mod.rs index 39c3413c..cb8f6a6b 100644 --- a/src/bors/mod.rs +++ b/src/bors/mod.rs @@ -25,6 +25,8 @@ mod handlers; pub mod merge_queue; pub mod mergeable_queue; +pub use command::CommandPrefix; + #[cfg(test)] pub static WAIT_FOR_CANCEL_TIMED_OUT_BUILDS_REFRESH: TestSyncMarker = TestSyncMarker::new(); diff --git a/src/github/server.rs b/src/github/server.rs index 09ff19bc..450ddfc7 100644 --- a/src/github/server.rs +++ b/src/github/server.rs @@ -5,7 +5,7 @@ use crate::bors::mergeable_queue::{ handle_mergeable_queue_item, }; use crate::bors::{ - BorsContext, RepositoryState, RollupMode, handle_bors_global_event, + BorsContext, CommandPrefix, RepositoryState, RollupMode, handle_bors_global_event, handle_bors_repository_event, }; use crate::github::webhook::GitHubWebhook; @@ -42,7 +42,7 @@ pub struct ServerState { webhook_secret: WebhookSecret, repositories: HashMap>, db: Arc, - cmd_prefix: String, + cmd_prefix: CommandPrefix, } impl ServerState { @@ -52,7 +52,7 @@ impl ServerState { webhook_secret: WebhookSecret, repositories: HashMap>, db: Arc, - cmd_prefix: String, + cmd_prefix: CommandPrefix, ) -> Self { Self { repository_event_queue, @@ -68,7 +68,7 @@ impl ServerState { &self.webhook_secret } - pub fn get_cmd_prefix(&self) -> &str { + pub fn get_cmd_prefix(&self) -> &CommandPrefix { &self.cmd_prefix } } @@ -123,7 +123,7 @@ async fn help_handler(State(state): State) -> impl IntoResponse HtmlTemplate(HelpTemplate { repos, - cmd_prefix: state.get_cmd_prefix().to_string(), + cmd_prefix: state.get_cmd_prefix().as_ref().to_string(), }) } diff --git a/src/tests/mocks/bors.rs b/src/tests/mocks/bors.rs index 4b9a54d7..f415cd34 100644 --- a/src/tests/mocks/bors.rs +++ b/src/tests/mocks/bors.rs @@ -19,8 +19,8 @@ use super::repository::PullRequest; use crate::bors::merge_queue::MergeQueueSender; use crate::bors::mergeable_queue::MergeableQueueSender; use crate::bors::{ - RollupMode, WAIT_FOR_CANCEL_TIMED_OUT_BUILDS_REFRESH, WAIT_FOR_MERGEABILITY_STATUS_REFRESH, - WAIT_FOR_PR_STATUS_REFRESH, WAIT_FOR_WORKFLOW_STARTED, + CommandPrefix, RollupMode, WAIT_FOR_CANCEL_TIMED_OUT_BUILDS_REFRESH, + WAIT_FOR_MERGEABILITY_STATUS_REFRESH, WAIT_FOR_PR_STATUS_REFRESH, WAIT_FOR_WORKFLOW_STARTED, }; use crate::database::{BuildStatus, DelegatedPermission, OctocrabMergeableState, PullRequestModel}; use crate::github::api::load_repositories; @@ -33,9 +33,6 @@ use crate::tests::mocks::workflow::{ }; use octocrab::params::checks::{CheckRunConclusion, CheckRunStatus}; -pub fn default_cmd_prefix() -> String { - "@bors".to_string() -} use crate::tests::mocks::{ Branch, ExternalHttpMock, GitHubState, Repo, User, default_pr_number, default_repo_name, }; @@ -46,6 +43,10 @@ use crate::{ create_app, create_bors_process, }; +pub fn default_cmd_prefix() -> CommandPrefix { + "@bors".to_string().into() +} + pub struct BorsBuilder { github: GitHubState, pool: PgPool, @@ -131,7 +132,7 @@ impl BorsTester { } let ctx = BorsContext::new( - CommandParser::new("@bors".to_string()), + CommandParser::new("@bors".to_string().into()), db.clone(), repos.clone(), );