Merge pull request #993 from rylev/post-merge-improvements

Post merge improvements

Author: rylev

site/src/github.rs

@@ -1,5 +1,5 @@
 use crate::api::github::Issue;
-use crate::comparison::{ComparisonSummary, Direction};
+use crate::comparison::{ComparisonConfidence, ComparisonSummary, Direction};
 use crate::load::{Config, SiteCtxt, TryCommit};
 
 use anyhow::Context as _;
@@ -560,75 +560,97 @@ async fn post_comparison_comment(ctxt: &SiteCtxt, commit: QueuedCommit, is_maste
         "https://perf.rust-lang.org/compare.html?start={}&end={}",
         commit.parent_sha, commit.sha
     );
-    let (summary, direction) =
-        categorize_benchmark(commit.sha.clone(), commit.parent_sha, ctxt).await;
+    let (summary, direction) = categorize_benchmark(
+        ctxt,
+        commit.sha.clone(),
+        commit.parent_sha,
+        is_master_commit,
+    )
+    .await;
+
+    let body = format!(
+        "Finished benchmarking commit ({sha}): [comparison url]({url}).
+
+**Summary**: {summary}
+{rest}",
+        sha = commit.sha,
+        url = comparison_url,
+        summary = summary,
+        rest = if is_master_commit {
+            master_run_body(direction)
+        } else {
+            try_run_body(direction)
+        }
+    );
+
+    post_comment(&ctxt.config, commit.pr, body).await;
+}
+
+fn master_run_body(direction: Option<Direction>) -> String {
     let label = match direction {
         Some(Direction::Regression | Direction::Mixed) => "+perf-regression",
         Some(Direction::Improvement) | None => "-perf-regression",
     };
-    let rollup_msg = if is_master_commit {
-        ""
-    } else {
-        "Benchmarking this pull request likely means that it is \
-perf-sensitive, so we're automatically marking it as not fit \
-for rolling up. "
-    };
-    let next_steps_msg = direction
-        .map(|d| {
-            format!(
-                "{}{}",
-                if is_master_commit {
-                    ""
-                } else {
-                    "While you can manually mark this PR as fit \
-            for rollup, we strongly recommend not doing so since this PR led to changes in \
-            compiler perf."
-                },
-                match d {
-                    Direction::Regression | Direction::Mixed =>
-                        "\n\n**Next Steps**: If you can justify the \
+    let next_steps = match direction {
+        Some(Direction::Regression | Direction::Mixed) => {
+            "\n\n**Next Steps**: If you can justify the \
                 regressions found in this perf run, please indicate this with \
                 `@rustbot label: +perf-regression-triaged` along with \
                 sufficient written justification. If you cannot justify the regressions \
-                please fix the regressions (either in this PR if it's not yet merged or \
-                in another PR), and then add the `perf-regression-triaged` label to this PR.",
-                    Direction::Improvement => "",
-                }
-            )
-        })
-        .unwrap_or(String::new());
-    let bors_msg = if is_master_commit {
-        ""
-    } else {
-        "@bors rollup=never\n"
+                please open an issue or create a new PR that fixes the regressions, \
+                add a comment linking to the newly created issue or PR, \
+                and then add the `perf-regression-triaged` label to this PR."
+        }
+        Some(Direction::Improvement) | None => "",
     };
-    post_comment(
-        &ctxt.config,
-        commit.pr,
-        format!(
-            "Finished benchmarking commit ({sha}): [comparison url]({url}).
 
-**Summary**: {summary}
+    format!(
+        "
+{next_steps} 
+
+@rustbot label: {label}",
+        next_steps = next_steps,
+        label = label
+    )
+}
+
+fn try_run_body(direction: Option<Direction>) -> String {
+    let label = match direction {
+        Some(Direction::Regression | Direction::Mixed) => "+perf-regression",
+        Some(Direction::Improvement) | None => "-perf-regression",
+    };
+    let next_steps = match direction {
+        Some(Direction::Regression | Direction::Mixed) => {
+            "\n\n**Next Steps**: If you can justify the regressions found in \
+            this try perf run, please indicate this with \
+            `@rustbot label: +perf-regression-triaged` along with \
+            sufficient written justification. If you cannot justify the regressions \
+            please fix the regressions and do another perf run. If the next run \
+            shows neutral or positive results, the label will be automatically removed."
+        }
+        Some(Direction::Improvement) | None => "",
+    };
 
-{rollup}{next_steps} 
-{bors}
+    format!(
+        "
+Benchmarking this pull request likely means that it is \
+perf-sensitive, so we're automatically marking it as not fit \
+for rolling up. While you can manually mark this PR as fit \
+for rollup, we strongly recommend not doing so since this PR led to changes in \
+compiler perf.{next_steps} 
+
+@bors rollup=never
 @rustbot label: +S-waiting-on-review -S-waiting-on-perf {label}",
-            sha = commit.sha,
-            url = comparison_url,
-            summary = summary,
-            rollup = rollup_msg,
-            next_steps = next_steps_msg,
-            bors = bors_msg,
-            label = label
-        ),
+        next_steps = next_steps,
+        label = label
     )
-    .await;
 }
 
 async fn categorize_benchmark(
+    ctxt: &SiteCtxt,
     commit_sha: String,
     parent_sha: String,
-    ctxt: &SiteCtxt,
+    is_master_commit: bool,
 ) -> (String, Option<Direction>) {
     let comparison = match crate::comparison::compare(
         collector::Bound::Commit(parent_sha),
@@ -645,7 +667,7 @@ async fn categorize_benchmark(
     please file an issue in [rust-lang/rustc-perf](https://github.com/rust-lang/rustc-perf/issues/new).";
     let (summary, (direction, magnitude)) =
         match ComparisonSummary::summarize_comparison(&comparison) {
-            Some(s) if s.confidence().is_atleast_probably_relevant() => {
+            Some(s) if comparison_is_relevant(s.confidence(), is_master_commit) => {
                 let direction_and_magnitude = s.direction_and_magnitude().unwrap();
                 (s, direction_and_magnitude)
             }
@@ -678,6 +700,16 @@ async fn categorize_benchmark(
     (result, Some(direction))
 }
 
+/// Whether a comparison is relevant enough to show
+fn comparison_is_relevant(confidence: ComparisonConfidence, is_master_commit: bool) -> bool {
+    if is_master_commit {
+        confidence.is_definitely_relevant()
+    } else {
+        // is try run
+        confidence.is_atleast_probably_relevant()
+    }
+}
+
 pub(crate) struct PullRequest {
     pub number: u64,
     pub title: String,