From 54d1d659641ba846ccd98cb4da714377606ccbbd Mon Sep 17 00:00:00 2001 From: Stefan Majewsky Date: Thu, 15 Aug 2024 14:45:15 +0200 Subject: [PATCH] convert find_reviews() into a method on type RepoChangeset --- src/changes.rs | 122 +++++++++++++++++++++++++++++++++++++++++++++--- src/main.rs | 123 ++++--------------------------------------------- src/util.rs | 11 +++++ 3 files changed, 137 insertions(+), 119 deletions(-) create mode 100644 src/util.rs diff --git a/src/changes.rs b/src/changes.rs index 6e68980..2af3105 100644 --- a/src/changes.rs +++ b/src/changes.rs @@ -1,19 +1,129 @@ +use std::sync::Arc; + +use crate::util::parse_remote; +use anyhow::{anyhow, Context}; +use octocrab::commits::PullRequestTarget; +use octocrab::models::commits::Commit; use octocrab::models::pulls::Review; use octocrab::models::pulls::ReviewState::Approved; +use octocrab::Octocrab; #[derive(Clone, Debug)] pub struct RepoChangeset { - pub name: String, - pub remote: String, + pub name: String, + pub remote: String, pub base_commit: String, pub head_commit: String, - pub changes: Vec, + pub changes: Vec, +} + +impl RepoChangeset { + pub async fn analyze_commits(&mut self, octocrab: &Arc) -> Result<(), anyhow::Error> { + let (repo_owner, repo_name) = parse_remote(&self.remote).context("while parsing remote")?; + + let compare = octocrab + .commits(repo_owner.clone(), repo_name.clone()) + .compare(&self.base_commit, &self.head_commit) + .send() + .await + .context(format!( + "failed to compare {}/compare/{}...{}", + self.remote.trim_end_matches(".git"), + &self.base_commit, + &self.head_commit + ))?; + + for commit in &compare.commits { + self.analyze_commit(octocrab, commit).await?; + } + + Ok(()) + } + + async fn analyze_commit(&mut self, octocrab: &Arc, commit: &Commit) -> Result<(), anyhow::Error> { + // TODO: it's not nice that we have to do this each time, this should be parsed once and + // stored inside of `self.remote` + let (repo_owner, repo_name) = parse_remote(&self.remote).context("while parsing remote")?; + + let mut associated_prs_page = octocrab + .commits(repo_owner.clone(), repo_name.clone()) + .associated_pull_requests(PullRequestTarget::Sha(commit.sha.clone())) + .send() + .await + .context("failed to get associated prs")?; + assert!( + associated_prs_page.next.is_none(), + "found more than one page for associated_prs" + ); + let associated_prs = associated_prs_page.take_items(); + + let change_commit = CommitMetadata { + headline: commit + .commit + .message + .split('\n') + .next() + .unwrap_or("") + .to_string(), + link: commit.html_url.clone(), + }; + + if associated_prs.is_empty() { + self.changes.push(Changeset { + commits: vec![change_commit], + pr_link: None, + approvals: Vec::new(), + }); + return Ok(()); + } + + for associated_pr in &associated_prs { + println!("pr number: {:}", associated_pr.number); + + let mut pr_reviews_page = octocrab + .pulls(repo_owner.clone(), repo_name.clone()) + .list_reviews(associated_pr.number) + .send() + .await + .context("failed to get reviews")?; + assert!( + pr_reviews_page.next.is_none(), + "found more than one page for associated_prs" + ); + let pr_reviews = pr_reviews_page.take_items(); + + let associated_pr_link = Some( + associated_pr + .html_url + .as_ref() + .ok_or(anyhow!("pr without an html link!?"))? + .to_string(), + ); + + if let Some(changeset) = self.changes.iter_mut().find(|cs| cs.pr_link == associated_pr_link) { + changeset.commits.push(change_commit.clone()); + changeset.collect_approved_reviews(&pr_reviews); + continue; + } + + let mut changeset = Changeset { + commits: vec![change_commit.clone()], + pr_link: associated_pr_link, + approvals: Vec::new(), + }; + + changeset.collect_approved_reviews(&pr_reviews); + self.changes.push(changeset); + } + + Ok(()) + } } #[derive(Clone, Debug)] pub struct Changeset { - pub commits: Vec, - pub pr_link: Option, + pub commits: Vec, + pub pr_link: Option, pub approvals: Vec, } @@ -35,5 +145,5 @@ impl Changeset { #[derive(Clone, Debug)] pub struct CommitMetadata { pub headline: String, - pub link: String, + pub link: String, } diff --git a/src/main.rs b/src/main.rs index f2adea4..a7bd4cb 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2,20 +2,18 @@ mod changes; mod helm_config; +mod util; -use std::sync::Arc; use std::{env, str}; use anyhow::{anyhow, Context}; -use changes::{Changeset, CommitMetadata, RepoChangeset}; +use changes::RepoChangeset; use clap::builder::styling::Style; use clap::{Parser, Subcommand}; use git2::Repository; use helm_config::ImageRefs; use lazy_static::lazy_static; -use octocrab::commits::PullRequestTarget; use octocrab::Octocrab; -use url::Url; const BOLD_UNDERLINE: Style = Style::new().bold().underline(); lazy_static! { @@ -89,13 +87,13 @@ async fn main() -> Result<(), anyhow::Error> { match &cli.command { Commands::Repo { remote } => { let repo = &mut RepoChangeset { - name: parse_remote(remote).context("while parsing remote")?.1, - remote: remote.clone(), + name: util::parse_remote(remote).context("while parsing remote")?.1, + remote: remote.clone(), base_commit: cli.base, head_commit: cli.head, - changes: Vec::new(), + changes: Vec::new(), }; - find_reviews(&octocrab, repo).await.context("while finding reviews")?; + repo.analyze_commits(&octocrab).await.context("while finding reviews")?; print_changes(&[repo.clone()]); }, Commands::HelmChart { workspace } => { @@ -103,7 +101,7 @@ async fn main() -> Result<(), anyhow::Error> { find_values_yaml(workspace.clone(), &cli.base, &cli.head).context("while finding values.yaml files")?; for repo in &mut changes { - find_reviews(&octocrab, repo) + repo.analyze_commits(&octocrab) .await .context("while collecting repo changes")?; } @@ -115,15 +113,6 @@ async fn main() -> Result<(), anyhow::Error> { Ok(()) } -fn parse_remote(remote: &str) -> Result<(String, String), anyhow::Error> { - let remote_url = Url::parse(remote).context("can't parse remote")?; - let repo_parts: Vec<&str> = remote_url.path().trim_start_matches('/').split('/').collect(); - let repo_owner = repo_parts[0].to_string(); - let repo_name = repo_parts[1].trim_end_matches(".git").to_string(); - - Ok((repo_owner, repo_name)) -} - fn find_values_yaml(workspace: String, base: &str, head: &str) -> Result, anyhow::Error> { let repo = Repository::open(workspace).context("failed to open repository")?; @@ -170,12 +159,12 @@ fn find_values_yaml(workspace: String, base: &str, head: &str) -> Result Result, repo: &mut RepoChangeset) -> Result<(), anyhow::Error> { - let (repo_owner, repo_name) = parse_remote(&repo.remote).context("while parsing remote")?; - - let compare = octocrab - .commits(repo_owner.clone(), repo_name.clone()) - .compare(&repo.base_commit, &repo.head_commit) - .send() - .await - .context(format!( - "failed to compare {}/compare/{}...{}", - repo.remote.trim_end_matches(".git"), - &repo.base_commit, - &repo.head_commit - ))?; - - for commit in &compare.commits { - let mut associated_prs_page = octocrab - .commits(repo_owner.clone(), repo_name.clone()) - .associated_pull_requests(PullRequestTarget::Sha(commit.sha.clone())) - .send() - .await - .context("failed to get associated prs")?; - assert!( - associated_prs_page.next.is_none(), - "found more than one page for associated_prs" - ); - let associated_prs = associated_prs_page.take_items(); - - let change_commit = CommitMetadata { - headline: commit - .commit - .message - .split('\n') - .next() - .unwrap_or("") - .to_string(), - link: commit.html_url.clone(), - }; - - if associated_prs.is_empty() { - repo.changes.push(Changeset { - commits: vec![change_commit], - pr_link: None, - approvals: Vec::new(), - }); - continue; - } - - for associated_pr in &associated_prs { - println!("pr number: {:}", associated_pr.number); - - let mut pr_reviews_page = octocrab - .pulls(repo_owner.clone(), repo_name.clone()) - .list_reviews(associated_pr.number) - .send() - .await - .context("failed to get reviews")?; - assert!( - pr_reviews_page.next.is_none(), - "found more than one page for associated_prs" - ); - let pr_reviews = pr_reviews_page.take_items(); - - let associated_pr_link = Some( - associated_pr - .html_url - .as_ref() - .ok_or(anyhow!("pr without an html link!?"))? - .to_string(), - ); - - if let Some(changeset) = repo.changes.iter_mut().find(|cs| cs.pr_link == associated_pr_link) { - changeset.commits.push(change_commit.clone()); - changeset.collect_approved_reviews(&pr_reviews); - continue; - } - - let mut changeset = Changeset { - commits: vec![change_commit.clone()], - pr_link: associated_pr_link, - approvals: Vec::new(), - }; - - changeset.collect_approved_reviews(&pr_reviews); - repo.changes.push(changeset); - } - } - - Ok(()) -} - fn print_changes(changes: &[RepoChangeset]) { for change in changes { println!( diff --git a/src/util.rs b/src/util.rs new file mode 100644 index 0000000..f972f45 --- /dev/null +++ b/src/util.rs @@ -0,0 +1,11 @@ +use anyhow::Context; +use url::Url; + +pub fn parse_remote(remote: &str) -> Result<(String, String), anyhow::Error> { + let remote_url = Url::parse(remote).context("can't parse remote")?; + let repo_parts: Vec<&str> = remote_url.path().trim_start_matches('/').split('/').collect(); + let repo_owner = repo_parts[0].to_string(); + let repo_name = repo_parts[1].trim_end_matches(".git").to_string(); + + Ok((repo_owner, repo_name)) +}