diff --git a/Cargo.lock b/Cargo.lock index 7293f24..db5ade5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -786,8 +786,7 @@ dependencies = [ [[package]] name = "octocrab" version = "0.39.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9305e4c99543ecd0f42bd659c9e9d6ca7115fe5e37d5c85a7277b1db0d4c4101" +source = "git+https://github.com/SuperSandro2000/octocrab.git?branch=pr_commits#24575e78ec3a3449dd042cf4be46a7faad3ad430" dependencies = [ "arc-swap", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index 0e14692..bec8b1e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,7 +9,8 @@ edition = "2021" anyhow = "^1" clap = { version = "^4", features = ["derive", "env"] } git2 = "^0" -octocrab = "^0" +# change back to crates.io after https://github.com/XAMPPRocky/octocrab/pull/680 got released +octocrab = { version = "^0", git = "https://github.com/SuperSandro2000/octocrab.git", branch = "pr_commits" } serde = "^1" serde_yml = "^0" tokio = { version = "^1", features = ["macros", "rt-multi-thread"] } diff --git a/src/changes.rs b/src/changes.rs index 104518a..e16aa13 100644 --- a/src/changes.rs +++ b/src/changes.rs @@ -85,9 +85,26 @@ impl RepoChangeset { .to_string(), ); + let mut pr_commits_page = octocrab + .pulls(&self.remote.owner, &self.remote.repository) + .pr_commits(associated_pr.number) + .await + .context("failed to get pr commits")?; + assert!( + pr_commits_page.next.is_none(), + "found more than one page for associated_prs" + ); + + let pr_commits = pr_commits_page.take_items(); + assert!( + pr_commits.len() <= 250, + "found more than 250 commits which requires a different api endpoint per doc" + ); + let head_sha = pr_commits.last().ok_or(anyhow!("PR contains no commits?"))?.sha.clone(); + 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); + changeset.collect_approved_reviews(&pr_reviews, &head_sha); continue; } @@ -97,7 +114,7 @@ impl RepoChangeset { approvals: Vec::new(), }; - changeset.collect_approved_reviews(&pr_reviews); + changeset.collect_approved_reviews(&pr_reviews, &head_sha); self.changes.push(changeset); } @@ -114,7 +131,7 @@ pub struct Changeset { impl Changeset { // pr_reviews must be sorted by key submitted_at! - pub fn collect_approved_reviews(&mut self, pr_reviews: &[Review]) { + pub fn collect_approved_reviews(&mut self, pr_reviews: &[Review], head_sha: &String) { let mut last_review_by: Vec<&String> = vec![]; // reverse the order of reviews to start with the oldest @@ -123,12 +140,20 @@ impl Changeset { continue; }; - // only consider the last review of any user + // Only consider the last review of any user. + // For example a user might have requested changes early on in the PR and later approved it + // or requested additional changes after supplying an approval first. if last_review_by.contains(&&user.login) { continue; } last_review_by.push(&user.login); + // Only account for reviews done on the last commit of the PR. + // We could count the PR as partly reviewed but that is to complicated to present at the moment. + if pr_review.commit_id != Some(head_sha.to_string()) { + continue; + } + // in case it isn't approve, ignore it if pr_review.state != Some(Approved) { continue;