Improve CI

.github/workflows/main.yml

@@ -1,17 +1,34 @@
-name: Build Rust
+name: CI
 
 on: [push, pull_request]
 
 jobs:
-  build:
-    name: Build Rust
+  test:
+    name: Test
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
-      - uses: hecrj/setup-rust-action@v1.3.4
+      - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
+      - run: cargo test
+
+  format:
+    name: Format
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
+      - run: cargo fmt --all -- --check
+
+  lint:
+    name: Lint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
+      - uses: clechasseur/rs-clippy-check@80bcfb7d14c9bb0415d025602b7fabcf9463becd # v3.0.4
         with:
-          rust-version: 1.58
-      - name: Build
-        run: cargo build
-      - name: Test
-        run: cargo test
+          args: -- -D warnings
+
+  check:
+    name: Check
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
+      - run: cargo check

src/github.rs

@@ -51,7 +51,7 @@ pub struct GithubReview {
 }
 
 impl GithubPullRequest {
-    pub async fn list(repository: String, token: &str) -> Result<Vec<GithubPullRequest>> {
+    pub async fn list(repository: &str, token: &str) -> Result<Vec<GithubPullRequest>> {
         let api_url = format!(
             "https://api.github.com/repos/{}/pulls?state=open&per_page=100",
             repository

src/google.rs

@@ -9,34 +9,31 @@ pub struct GoogleChatMessage {
 
 impl GoogleChatMessage {
     pub fn from(text: String) -> GoogleChatMessage {
-        GoogleChatMessage { text: text }
+        GoogleChatMessage { text }
     }
 
     /// Build a webhook URL with threadKey set to the provided value, and messageReplyOption set to allow threading.
-    pub fn build_webhook_url(webhook_url: &String, thread_key: &String) -> Result<String> {
+    pub fn build_webhook_url(webhook_url: &str, thread_key: &str) -> Result<String> {
         let url = Url::parse(webhook_url)?;
         // strip any existing threadKey / messageReplyOption parameters
         let other_params = url
             .query_pairs()
             .filter(|(name, _)| name.ne("threadKey") && name.ne("messageReplyOption"));
         // rebuild URL with the provided threadKey value, and our desired messageReplyOption
         let mut updated_url = url.clone();
-        updated_url.query_pairs_mut()
+        updated_url
+            .query_pairs_mut()
             .clear()
             .extend_pairs(other_params)
             .extend_pairs(&[
-                (&"messageReplyOption".to_string(), &"REPLY_MESSAGE_FALLBACK_TO_NEW_THREAD".to_string()),
-                (&"threadKey".to_string(), thread_key)
+                ("messageReplyOption", "REPLY_MESSAGE_FALLBACK_TO_NEW_THREAD"),
+                ("threadKey", thread_key),
             ]);
         Ok(updated_url.as_str().to_string())
     }
 
-    pub async fn send(
-        self,
-        webhook_url: &String,
-        thread_key: &String,
-    ) -> Result<GoogleChatMessage> {
-        let url = GoogleChatMessage::build_webhook_url(&webhook_url, &thread_key)?;
+    pub async fn send(self, webhook_url: &str, thread_key: &str) -> Result<GoogleChatMessage> {
+        let url = GoogleChatMessage::build_webhook_url(webhook_url, thread_key)?;
 
         let response = reqwest::Client::new()
             .post(url.to_string())
@@ -61,25 +58,32 @@ mod tests {
 
     #[test]
     fn test_build_threaded_webhook_with_placeholder() {
-        let result = GoogleChatMessage::build_webhook_url(&String::from("https://example.com/ABCDEF?threadKey={threadKey}"), &String::from("1234"));
+        let result = GoogleChatMessage::build_webhook_url(
+            "https://example.com/ABCDEF?threadKey={threadKey}",
+            "1234",
+        );
         assert_eq!(Result::unwrap(result), String::from("https://example.com/ABCDEF?messageReplyOption=REPLY_MESSAGE_FALLBACK_TO_NEW_THREAD&threadKey=1234"))
     }
 
     #[test]
     fn test_build_threaded_webhook_without_placeholder() {
-        let result = GoogleChatMessage::build_webhook_url(&String::from("https://example.com/ABCDEF"), &String::from("1234"));
+        let result = GoogleChatMessage::build_webhook_url("https://example.com/ABCDEF", "1234");
         assert_eq!(Result::unwrap(result), String::from("https://example.com/ABCDEF?messageReplyOption=REPLY_MESSAGE_FALLBACK_TO_NEW_THREAD&threadKey=1234"))
     }
 
     #[test]
     fn test_preserves_other_existing_params() {
-        let result = GoogleChatMessage::build_webhook_url(&String::from("https://example.com/ABCDEF?foo=bar"), &String::from("1234"));
+        let result =
+            GoogleChatMessage::build_webhook_url("https://example.com/ABCDEF?foo=bar", "1234");
         assert_eq!(Result::unwrap(result), String::from("https://example.com/ABCDEF?foo=bar&messageReplyOption=REPLY_MESSAGE_FALLBACK_TO_NEW_THREAD&threadKey=1234"))
     }
 
     #[test]
     fn test_overrides_existing_messagereplyoption_parameter() {
-        let result = GoogleChatMessage::build_webhook_url(&String::from("https://example.com/ABCDEF?messageReplyOption=REPLY_MESSAGE"), &String::from("1234"));
+        let result = GoogleChatMessage::build_webhook_url(
+            "https://example.com/ABCDEF?messageReplyOption=REPLY_MESSAGE",
+            "1234",
+        );
         assert_eq!(Result::unwrap(result), String::from("https://example.com/ABCDEF?messageReplyOption=REPLY_MESSAGE_FALLBACK_TO_NEW_THREAD&threadKey=1234"))
     }
-}
+}

src/main.rs

@@ -12,15 +12,15 @@ use github::GithubPullRequest;
 use google::GoogleChatMessage;
 
 async fn scan_repository(
-    repository_name: String,
-    github_token: &String,
-    ignored_users: &Vec<&str>,
+    repository_name: &str,
+    github_token: &str,
+    ignored_users: &[&str],
     announced_users: &Option<Vec<usize>>,
-    ignored_labels: &Vec<&str>,
+    ignored_labels: &[&str],
 ) -> Result<Vec<GithubPullRequest>, Error> {
     info!("Starting PR scan of {}", repository_name);
 
-    let pull_requests = GithubPullRequest::list(repository_name, &github_token).await?;
+    let pull_requests = GithubPullRequest::list(repository_name, github_token).await?;
     let mut pull_requests_to_review: Vec<GithubPullRequest> = vec![];
 
     info!("Found {} PR's", pull_requests.len());
@@ -53,7 +53,7 @@ async fn scan_repository(
         }
 
         if let Some(announced_users) = announced_users {
-            if !announced_users.contains(&pull_request.user().id()) {
+            if !announced_users.contains(pull_request.user().id()) {
                 info!("Users to announce: {:?}", announced_users);
                 info!(
                     "Ignoring PR {}({}) as it was raised by a user not included in the announced users list {}({})",
@@ -84,7 +84,7 @@ async fn scan_repository(
             continue;
         }
 
-        let pull_request_reviews = pull_request.reviews(&github_token).await?;
+        let pull_request_reviews = pull_request.reviews(github_token).await?;
 
         info!(
             "Found {} reviews for PR {}({})",
@@ -122,23 +122,22 @@ async fn main() -> Result<(), Error> {
 
     let repositories: String =
         env::var("GITHUB_REPOSITORIES").context("GITHUB_REPOSITORIES must be set")?;
-    let repositories: Vec<&str> = repositories.split(",").collect();
+    let repositories: Vec<&str> = repositories.split(',').collect();
     let github_token: String = env::var("GITHUB_TOKEN").context("GITHUB_TOKEN must be set")?;
     let webhook_url: String =
         env::var("GOOGLE_WEBHOOK_URL").context("GOOGLE_WEBHOOK_URL must be set")?;
     let ignored_users: String = env::var("GITHUB_IGNORED_USERS").unwrap_or("".to_string());
-    let ignored_users: Vec<&str> = ignored_users.split(",").collect();
-    let announced_users: Option<Vec<usize>> = env::var("GITHUB_ANNOUNCED_USERS")
-        .ok()
-        .and_then(|s| {
+    let ignored_users: Vec<&str> = ignored_users.split(',').collect();
+    let announced_users: Option<Vec<usize>> =
+        env::var("GITHUB_ANNOUNCED_USERS").ok().and_then(|s| {
             if s.is_empty() {
                 None
             } else {
                 Some(s.split(',').map(|id| id.parse().unwrap()).collect())
             }
         });
     let ignored_labels: String = env::var("GITHUB_IGNORED_LABELS").unwrap_or("".to_string());
-    let ignored_labels: Vec<&str> = ignored_labels.split(",").collect();
+    let ignored_labels: Vec<&str> = ignored_labels.split(',').collect();
     let show_pr_age: bool = env::var("SHOW_PR_AGE")
         .map(|v| v == "true")
         .unwrap_or(false);
@@ -148,7 +147,7 @@ async fn main() -> Result<(), Error> {
     for repository in repositories {
         pull_requests_to_review.append(
             &mut scan_repository(
-                repository.to_string(),
+                repository,
                 &github_token,
                 &ignored_users,
                 &announced_users,