From 141f0c212d10b2cedbeacfe39e5c2c5f0c8f083e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 23 Aug 2022 10:43:35 +0200 Subject: [PATCH 1/9] Refactor --- tools/automator/src/pull_requests.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/automator/src/pull_requests.rs b/tools/automator/src/pull_requests.rs index 4fa04e787..baee75eed 100644 --- a/tools/automator/src/pull_requests.rs +++ b/tools/automator/src/pull_requests.rs @@ -18,6 +18,8 @@ impl PullRequest { let mut page = 1u32; 'outer: loop { + const MAX_RESULTS_PER_PAGE: u8 = 100; + println!("Fetching page {}...", page); let pull_request_page = octocrab::instance() .pulls("hannobraun", "Fornjot") @@ -25,7 +27,7 @@ impl PullRequest { .state(State::Closed) .sort(Sort::Updated) .direction(Direction::Descending) - .per_page(100) // this is the maximum number of results per page + .per_page(MAX_RESULTS_PER_PAGE) .page(page) .send() .await?; From 8652fd9ff8b2b8cc337b64cf6daf6ab5ddd96ad3 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 23 Aug 2022 10:46:17 +0200 Subject: [PATCH 2/9] Add comment --- tools/automator/src/pull_requests.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/automator/src/pull_requests.rs b/tools/automator/src/pull_requests.rs index baee75eed..28aef4dd8 100644 --- a/tools/automator/src/pull_requests.rs +++ b/tools/automator/src/pull_requests.rs @@ -25,6 +25,9 @@ impl PullRequest { .pulls("hannobraun", "Fornjot") .list() .state(State::Closed) + // It would be *much* better to sort by the date the pull + // requests were merged, since "updated" could result in false + // positives. GitHub doesn't support that though. .sort(Sort::Updated) .direction(Direction::Descending) .per_page(MAX_RESULTS_PER_PAGE) From bda037b74ddba5b3201c737d7b5ff334a1d820a0 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 23 Aug 2022 11:00:20 +0200 Subject: [PATCH 3/9] Refactor --- tools/automator/src/announcement.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/automator/src/announcement.rs b/tools/automator/src/announcement.rs index 90031d5bb..416095b94 100644 --- a/tools/automator/src/announcement.rs +++ b/tools/automator/src/announcement.rs @@ -52,9 +52,10 @@ async fn generate_announcement( ) -> anyhow::Result<()> { let mut pull_request_links = String::new(); - for PullRequest { number, html_url } in pull_requests { - let link = format!("[#{number}]: {html_url}\n"); + for pull_request in pull_requests { + let PullRequest { number, html_url } = pull_request; + let link = format!("[#{number}]: {html_url}\n"); pull_request_links.push_str(&link); } From 1a12cce367864f1387080ecd530da7dfd3dd17ab Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 23 Aug 2022 11:07:16 +0200 Subject: [PATCH 4/9] Add title to `PullRequest` --- tools/automator/src/announcement.rs | 4 +++- tools/automator/src/pull_requests.rs | 10 +++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/tools/automator/src/announcement.rs b/tools/automator/src/announcement.rs index 416095b94..eaeeab09e 100644 --- a/tools/automator/src/announcement.rs +++ b/tools/automator/src/announcement.rs @@ -53,7 +53,9 @@ async fn generate_announcement( let mut pull_request_links = String::new(); for pull_request in pull_requests { - let PullRequest { number, html_url } = pull_request; + let PullRequest { + number, html_url, .. + } = pull_request; let link = format!("[#{number}]: {html_url}\n"); pull_request_links.push_str(&link); diff --git a/tools/automator/src/pull_requests.rs b/tools/automator/src/pull_requests.rs index 28aef4dd8..517489d64 100644 --- a/tools/automator/src/pull_requests.rs +++ b/tools/automator/src/pull_requests.rs @@ -7,6 +7,7 @@ use url::Url; pub struct PullRequest { pub number: u64, + pub title: String, pub html_url: Url, } @@ -50,12 +51,19 @@ impl PullRequest { if let Some(merged_at) = pull_request.merged_at { if merged_at.date() >= last_release_date { let number = pull_request.number; + let title = pull_request.title.ok_or_else(|| { + anyhow!("Pull request is missing title") + })?; let html_url = pull_request.html_url.ok_or_else(|| { anyhow!("Pull request is missing URL") })?; - let pull_request = Self { number, html_url }; + let pull_request = Self { + number, + title, + html_url, + }; pull_requests.insert(pull_request.number, pull_request); } From 085b56ddbf973601afe9c6191573e7a080b613fd Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 23 Aug 2022 11:07:51 +0200 Subject: [PATCH 5/9] Generate list of pull requests --- tools/automator/src/announcement.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tools/automator/src/announcement.rs b/tools/automator/src/announcement.rs index eaeeab09e..12dfeed0a 100644 --- a/tools/automator/src/announcement.rs +++ b/tools/automator/src/announcement.rs @@ -50,13 +50,19 @@ async fn generate_announcement( pull_requests: impl IntoIterator, file: &mut File, ) -> anyhow::Result<()> { + let mut pull_request_list = String::new(); let mut pull_request_links = String::new(); for pull_request in pull_requests { let PullRequest { - number, html_url, .. + number, + title, + html_url, } = pull_request; + let item = format!("- {title} ([#{number}])\n"); + pull_request_list.push_str(&item); + let link = format!("[#{number}]: {html_url}\n"); pull_request_links.push_str(&link); } @@ -105,6 +111,12 @@ Improvements that are relevant to developers working on Fornjot itself. **TASK: Add internal improvements.** +### Unsorted pull requests + +**TASK: Sort into the categories above; update/merge as appropriate.** + +{pull_request_list} + ### Issue of the Week **TASK: Write.** From 98d4ce3b493e18abd4a6a481b80833bd67714621 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 23 Aug 2022 11:15:00 +0200 Subject: [PATCH 6/9] Simplify struct field name --- tools/automator/src/announcement.rs | 8 ++------ tools/automator/src/pull_requests.rs | 15 +++++---------- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/tools/automator/src/announcement.rs b/tools/automator/src/announcement.rs index 12dfeed0a..49247f380 100644 --- a/tools/automator/src/announcement.rs +++ b/tools/automator/src/announcement.rs @@ -54,16 +54,12 @@ async fn generate_announcement( let mut pull_request_links = String::new(); for pull_request in pull_requests { - let PullRequest { - number, - title, - html_url, - } = pull_request; + let PullRequest { number, title, url } = pull_request; let item = format!("- {title} ([#{number}])\n"); pull_request_list.push_str(&item); - let link = format!("[#{number}]: {html_url}\n"); + let link = format!("[#{number}]: {url}\n"); pull_request_links.push_str(&link); } diff --git a/tools/automator/src/pull_requests.rs b/tools/automator/src/pull_requests.rs index 517489d64..385547a88 100644 --- a/tools/automator/src/pull_requests.rs +++ b/tools/automator/src/pull_requests.rs @@ -8,7 +8,7 @@ use url::Url; pub struct PullRequest { pub number: u64, pub title: String, - pub html_url: Url, + pub url: Url, } impl PullRequest { @@ -54,16 +54,11 @@ impl PullRequest { let title = pull_request.title.ok_or_else(|| { anyhow!("Pull request is missing title") })?; - let html_url = - pull_request.html_url.ok_or_else(|| { - anyhow!("Pull request is missing URL") - })?; + let url = pull_request.html_url.ok_or_else(|| { + anyhow!("Pull request is missing URL") + })?; - let pull_request = Self { - number, - title, - html_url, - }; + let pull_request = Self { number, title, url }; pull_requests.insert(pull_request.number, pull_request); } From 12aa503dd7a1f7279707aee58f1a0f257b71f876 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 23 Aug 2022 11:34:18 +0200 Subject: [PATCH 7/9] Add author to `PullRequest` --- tools/automator/src/announcement.rs | 4 ++- tools/automator/src/pull_requests.rs | 49 +++++++++++++++++++++++----- 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/tools/automator/src/announcement.rs b/tools/automator/src/announcement.rs index 49247f380..ce1a74ce4 100644 --- a/tools/automator/src/announcement.rs +++ b/tools/automator/src/announcement.rs @@ -54,7 +54,9 @@ async fn generate_announcement( let mut pull_request_links = String::new(); for pull_request in pull_requests { - let PullRequest { number, title, url } = pull_request; + let PullRequest { + number, title, url, .. + } = pull_request; let item = format!("- {title} ([#{number}])\n"); pull_request_list.push_str(&item); diff --git a/tools/automator/src/pull_requests.rs b/tools/automator/src/pull_requests.rs index 385547a88..5d8c3a08d 100644 --- a/tools/automator/src/pull_requests.rs +++ b/tools/automator/src/pull_requests.rs @@ -2,13 +2,17 @@ use std::collections::BTreeMap; use anyhow::anyhow; use chrono::{Date, Utc}; -use octocrab::params::{pulls::Sort, Direction, State}; +use octocrab::{ + models::pulls::PullRequest as OctoPullRequest, + params::{pulls::Sort, Direction, State}, +}; use url::Url; pub struct PullRequest { pub number: u64, pub title: String, pub url: Url, + pub author: Author, } impl PullRequest { @@ -51,14 +55,22 @@ impl PullRequest { if let Some(merged_at) = pull_request.merged_at { if merged_at.date() >= last_release_date { let number = pull_request.number; - let title = pull_request.title.ok_or_else(|| { - anyhow!("Pull request is missing title") - })?; - let url = pull_request.html_url.ok_or_else(|| { - anyhow!("Pull request is missing URL") - })?; + let title = + pull_request.title.clone().ok_or_else(|| { + anyhow!("Pull request is missing title") + })?; + let url = + pull_request.html_url.clone().ok_or_else(|| { + anyhow!("Pull request is missing URL") + })?; + let author = Author::from_pull_request(&pull_request)?; - let pull_request = Self { number, title, url }; + let pull_request = Self { + number, + title, + url, + author, + }; pull_requests.insert(pull_request.number, pull_request); } @@ -75,3 +87,24 @@ impl PullRequest { Ok(pull_requests) } } + +pub struct Author { + pub name: String, + pub profile: Url, +} + +impl Author { + pub fn from_pull_request( + pull_request: &OctoPullRequest, + ) -> anyhow::Result { + let user = pull_request + .user + .clone() + .ok_or_else(|| anyhow!("Pull request is missing author"))?; + + let name = user.login; + let profile = user.html_url; + + Ok(Self { name, profile }) + } +} From f1c845497eb0a804edc52bcc9bd389f285aae3fd Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 23 Aug 2022 11:34:56 +0200 Subject: [PATCH 8/9] Add dependency on `map-macro` to `automator` --- Cargo.lock | 1 + tools/automator/Cargo.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index a10682b85..751c938eb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -219,6 +219,7 @@ dependencies = [ "anyhow", "chrono", "clap", + "map-macro", "octocrab", "tokio", "url", diff --git a/tools/automator/Cargo.toml b/tools/automator/Cargo.toml index 5ab1d8ed4..577c45c21 100644 --- a/tools/automator/Cargo.toml +++ b/tools/automator/Cargo.toml @@ -7,6 +7,7 @@ publish = false [dependencies] anyhow = "1.0.62" chrono = "0.4.22" +map-macro = "0.2.4" octocrab = "0.17.0" url = "2.2.2" From d5179cf45457af26e700d417dd2efbc04400f757 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 23 Aug 2022 11:35:19 +0200 Subject: [PATCH 9/9] Generate list of author links --- tools/automator/src/announcement.rs | 34 +++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/tools/automator/src/announcement.rs b/tools/automator/src/announcement.rs index ce1a74ce4..d0805ad7c 100644 --- a/tools/automator/src/announcement.rs +++ b/tools/automator/src/announcement.rs @@ -1,13 +1,14 @@ -use std::{fmt::Write, path::PathBuf}; +use std::{collections::HashSet, fmt::Write, path::PathBuf}; use anyhow::Context; use chrono::{Date, Datelike, Utc}; +use map_macro::set; use tokio::{ fs::{self, File}, io::AsyncWriteExt, }; -use crate::pull_requests::PullRequest; +use crate::pull_requests::{Author, PullRequest}; pub async fn create_release_announcement( last_release_date: Date, @@ -52,17 +53,41 @@ async fn generate_announcement( ) -> anyhow::Result<()> { let mut pull_request_list = String::new(); let mut pull_request_links = String::new(); + let mut author_links = String::new(); + + let author_blacklist = set! { + "hannobraun", + "dependabot[bot]" + }; + let mut authors = HashSet::new(); for pull_request in pull_requests { let PullRequest { - number, title, url, .. + number, + title, + url, + author, } = pull_request; + let author = if authors.contains(&author.name) + || author_blacklist.contains(author.name.as_str()) + { + None + } else { + authors.insert(author.name.clone()); + Some(author) + }; + let item = format!("- {title} ([#{number}])\n"); pull_request_list.push_str(&item); let link = format!("[#{number}]: {url}\n"); pull_request_links.push_str(&link); + + if let Some(Author { name, profile }) = author { + let author_link = format!("[@{name}]: {profile}\n"); + author_links.push_str(&author_link); + } } let mut buf = String::new(); @@ -125,7 +150,8 @@ Improvements that are relevant to developers working on Fornjot itself. **TASK: Write.** -{pull_request_links}\ +{pull_request_links} +{author_links}\ " )?;