Skip to content

Commit

Permalink
[summary] Update existing comment (#2953)
Browse files Browse the repository at this point in the history
## Motivation

Right now the performance summary always "spams" the PR with comments, one for everytime the `Rust` workflow finishes successfully in CI.

## Proposal

Now that we have the summary attached to the performance summary CI job, we have a way of accessing the historical values for these things. So now we can just edit an existing comment instead of always creating a new one.

## Test Plan

Will be tested with workflow dispatch, like in previous PRs. You can see that there's only one comment, and it's edited (instead of just commenting again)

## Release Plan

- Nothing to do / These changes follow the usual release cycle.
  • Loading branch information
ndr-ds authored Nov 26, 2024
1 parent 44b312e commit 7a31be5
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 14 deletions.
55 changes: 48 additions & 7 deletions linera-summary/src/github.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ use octocrab::{
};
use tracing::info;

use crate::performance_summary::PR_COMMENT_HEADER;

const API_REQUEST_DELAY_MS: u64 = 100;
const IGNORED_JOB_PREFIXES: &[&str] = &["lint-", "check-outdated-cli-md"];

Expand Down Expand Up @@ -164,18 +166,57 @@ impl Github {
&self.context
}

pub async fn comment_on_pr(&self, body: String) -> Result<()> {
// Updates an existing comment or creates a new one in the PR.
pub async fn upsert_pr_comment(&self, body: String) -> Result<()> {
let issue_handler = self.octocrab.issues(
self.context.repository.owner.clone(),
self.context.repository.name.clone(),
);
let existing_comment_id = issue_handler
.list_comments(self.context.pr_number)
.send()
.await?
.items
.into_iter()
.find_map(|comment| {
if comment.user.login == "github-actions[bot]"
&& comment
.body
.is_some_and(|body| body.starts_with(PR_COMMENT_HEADER))
{
Some(comment.id)
} else {
None
}
});

// Always print the summary to stdout, as we'll use it to set the job summary in CI.
info!("Printing summary to stdout...");
println!("{}", body);

if !self.is_local {
if let Some(existing_comment_id) = existing_comment_id {
if self.is_local {
info!(
"Would have updated comment {} on PR {}, but is local",
existing_comment_id, self.context.pr_number
);
} else {
info!(
"Updating existing comment {} on PR {}",
existing_comment_id, self.context.pr_number
);
issue_handler
.update_comment(existing_comment_id, body)
.await?;
}
} else if self.is_local {
info!(
"Would have commented on PR {}, but is local",
self.context.pr_number
);
} else {
info!("Commenting on PR {}", self.context.pr_number);
self.octocrab
.issues(
self.context.repository.owner.clone(),
self.context.repository.name.clone(),
)
issue_handler
.create_comment(self.context.pr_number, body)
.await?;
}
Expand Down
2 changes: 1 addition & 1 deletion linera-summary/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ async fn run(options: SummaryOptions) -> Result<()> {
let tracked_workflows = options.workflows();
let github = Github::new(options.is_local(), options.pr_number())?;
let summary = PerformanceSummary::init(github, tracked_workflows).await?;
summary.comment_on_pr().await?;
summary.upsert_pr_comment().await?;
Ok(())
}

Expand Down
14 changes: 8 additions & 6 deletions linera-summary/src/performance_summary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ use serde::Serialize;

use crate::{ci_runtime_comparison::CiRuntimeComparison, github::Github};

pub const PR_COMMENT_HEADER: &str = "## Performance Summary for commit";

#[derive(Serialize)]
pub struct PerformanceSummary {
#[serde(skip_serializing)]
Expand Down Expand Up @@ -65,8 +67,8 @@ impl PerformanceSummary {
);

let mut markdown_content = format!(
"## Performance Summary for commit [{}]({})\n\n",
short_commit_hash, commit_url
"{} [{}]({})\n\n",
PR_COMMENT_HEADER, short_commit_hash, commit_url
);

markdown_content.push_str("### CI Runtime Comparison\n\n");
Expand Down Expand Up @@ -97,10 +99,10 @@ impl PerformanceSummary {
markdown_content
}

pub async fn comment_on_pr(&self) -> Result<()> {
// Updates an existing comment or creates a new one in the PR.
pub async fn upsert_pr_comment(&self) -> Result<()> {
self.github
.comment_on_pr(self.format_comment_body())
.await?;
Ok(())
.upsert_pr_comment(self.format_comment_body())
.await
}
}

0 comments on commit 7a31be5

Please sign in to comment.