Skip to content

Centralize comment constructor #404

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
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
2 changes: 1 addition & 1 deletion docs/development.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]`
Expand Down
2 changes: 1 addition & 1 deletion src/bors/command/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
240 changes: 219 additions & 21 deletions src/bors/comment.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
use itertools::Itertools;
use octocrab::models::workflows::Job;
use serde::Serialize;
use std::fmt::Write;
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},
Expand All @@ -31,9 +35,9 @@ pub enum CommentTag {
}

impl Comment {
pub fn new(text: String) -> Self {
fn new(text: impl Into<String>) -> Self {
Self {
text,
text: text.into(),
metadata: None,
}
}
Expand All @@ -51,13 +55,213 @@ 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=<priority>] [rollup=<never|iffy|maybe|always>]`: Approve this PR on your behalf
- Optionally, you can specify the `<priority>` of the PR and if it is eligible for rollups (`<rollup>)`.
- `r=<user> [p=<priority>] [rollup=<never|iffy|maybe|always>]`: Approve this PR on behalf of `<user>`
- Optionally, you can specify the `<priority>` of the PR and if it is eligible for rollups (`<rollup>)`.
- You can pass a comma-separated list of GitHub usernames.
- `r-`: Unapprove this PR
- `p=<priority>` or `priority=<priority>`: Set the priority of this PR
- `rollup=<never|iffy|maybe|always>`: Set the rollup status of the PR
- `rollup`: Short for `rollup=always`
- `rollup-`: Short for `rollup=maybe`
- `delegate=<try|review>`: 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=<parent>] [jobs=<jobs>]`: Start a try build.
- Optionally, you can specify a `<parent>` 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 `<jobs>` to run in the try build.
- `try cancel`: Cancel a running try build
- `info`: Get information about the current PR

## Repository management
- `treeclosed=<priority>`: Close the tree for PRs with priority less than `<priority>`
- `treeclosed-` or `treeopen`: Open the repository tree for merging

## Meta commands
- `ping`: Check if the bot is alive
- `help`: Print this help message
"#,
)
}

pub async fn info_comment(pr: &PullRequestModel, db: Arc<PgDbClient>) -> Comment {
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")
}

pub fn parse_command_failed_comment(
error: &CommandParseError,
bot_prefix: &CommandPrefix,
) -> Comment {
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 🏓!")
}

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<String>) -> 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")
}

pub fn unapproval_comment(head_sha: &CommitSha, cancel_message: Option<String>) -> 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,
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
Expand All @@ -83,35 +287,32 @@ 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<Item = String>) -> 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(
repo: &GithubRepoName,
commit_sha: CommitSha,
failed_workflows: Vec<FailedWorkflowRun>,
) -> Comment {
use std::fmt::Write;

let mut msg = format!(":broken_heart: Test for {commit_sha} failed");
let mut workflow_links = failed_workflows
.iter()
Expand Down Expand Up @@ -170,7 +371,6 @@ pub fn try_build_started_comment(
bot_prefix: &CommandPrefix,
cancelled_workflow_urls: Vec<String>,
) -> 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() {
Expand Down Expand Up @@ -241,11 +441,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 {
Expand Down Expand Up @@ -310,7 +510,6 @@ fn list_workflows_status(workflows: &[WorkflowModel]) -> String {
}
)
})
.collect::<Vec<_>>()
.join("\n")
}

Expand All @@ -330,7 +529,6 @@ pub fn auto_build_succeeded_comment(
let urls = workflows
.iter()
.map(|w| format!("[{}]({})", w.name, w.url))
.collect::<Vec<_>>()
.join(", ");

Comment::new(format!(
Expand Down
Loading
Loading