From 9fbf59c42a4c267204980955a1c70b80ee919964 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ben=20P=C3=BCschel?= Date: Mon, 30 Sep 2024 16:28:03 +0200 Subject: [PATCH] chore: Implement some style improvements (#698) * test(pr_review): remove deprecated function calls * style(docs): fix indentation Fix indentation and break-up large first paragraphs in doc comments. * style: assert `is_err()` instead of `!is_ok()` * style: implement `Display` instead of `ToString` Any Display implementation will automatically implement ToString, so this change is not breaking. --- src/api/commits/associated_pull_requests.rs | 12 +++++---- src/api/gists.rs | 6 ++--- src/params.rs | 2 ++ tests/pull_request_review_operations_test.rs | 28 +++++--------------- tests/user_blocks_tests.rs | 2 +- 5 files changed, 20 insertions(+), 30 deletions(-) diff --git a/src/api/commits/associated_pull_requests.rs b/src/api/commits/associated_pull_requests.rs index ecddcf7f..694e3e16 100644 --- a/src/api/commits/associated_pull_requests.rs +++ b/src/api/commits/associated_pull_requests.rs @@ -1,3 +1,5 @@ +use std::fmt::Display; + use super::*; /// helper to let users know they can pass a branch name or a commit sha @@ -8,11 +10,11 @@ pub enum PullRequestTarget { Sha(String), } -impl ToString for PullRequestTarget { - fn to_string(&self) -> String { +impl Display for PullRequestTarget { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::Branch(branch) => branch.to_string(), - Self::Sha(commit) => commit.to_string(), + Self::Branch(branch) => write!(f, "{}", branch), + Self::Sha(commit) => write!(f, "{}", commit), } } } @@ -59,7 +61,7 @@ impl<'octo, 'r> AssociatedPullRequestsBuilder<'octo, 'r> { "/repos/{owner}/{repo}/commits/{target}/pulls", owner = self.handler.owner, repo = self.handler.repo, - target = self.target.to_string(), + target = self.target, ); self.handler.crab.get(route, Some(&self)).await diff --git a/src/api/gists.rs b/src/api/gists.rs index dcb3bdfb..e591e993 100644 --- a/src/api/gists.rs +++ b/src/api/gists.rs @@ -39,11 +39,11 @@ impl<'octo> GistsHandler<'octo> { /// /// # Note /// * Calling with an authentication token will list all the gists of the - /// authenticated user + /// authenticated user /// /// * If no authentication token will list all the public gists from - /// GitHub's API. This can potentially produce a lot of results, so care is - /// advised. + /// GitHub's API. This can potentially produce a lot of results, so care is + /// advised. /// /// # Example /// diff --git a/src/params.rs b/src/params.rs index 450e073f..21acdf41 100644 --- a/src/params.rs +++ b/src/params.rs @@ -386,6 +386,7 @@ pub mod pulls { } /// Custom media types are used in the API to let consumers choose the + /// /// format of the data they wish to receive. This is done by adding one or /// more of the following types to the Accept header when you make a /// request. Media types are specific to resources, allowing them to change @@ -503,6 +504,7 @@ pub mod repos { } /// A Git reference of unknown type. + /// /// In some cases clients may have a string identifying a commit, but not /// know whether it's a branch or a tag or commit hash. /// Many Github APIs accept such strings. These APIs also accept `heads/` or `tags/`. diff --git a/tests/pull_request_review_operations_test.rs b/tests/pull_request_review_operations_test.rs index 06ac116f..d55a2a40 100644 --- a/tests/pull_request_review_operations_test.rs +++ b/tests/pull_request_review_operations_test.rs @@ -93,50 +93,38 @@ async fn should_work_with_specific_review() { let result = client .pulls(OWNER, REPO) - .pull_number(PULL_NUMBER) - .reviews() - .review(REVIEW_ID) + .pr_review_actions(PULL_NUMBER, REVIEW_ID) .get() .await; assert_eq!(result.unwrap(), review_ops_response); let result = client .pulls(OWNER, REPO) - .pull_number(PULL_NUMBER) - .reviews() - .review(REVIEW_ID) + .pr_review_actions(PULL_NUMBER, REVIEW_ID) .update("test") .await; assert_eq!(result.unwrap(), review_ops_response); let result = client .pulls(OWNER, REPO) - .pull_number(PULL_NUMBER) - .reviews() - .review(REVIEW_ID) + .pr_review_actions(PULL_NUMBER, REVIEW_ID) .delete_pending() .await; assert_eq!(result.unwrap(), review_ops_response); let result = client .pulls(OWNER, REPO) - .pull_number(PULL_NUMBER) - .reviews() - .review(REVIEW_ID) + .pr_review_actions(PULL_NUMBER, REVIEW_ID) .submit(ReviewAction::Comment, "test") .await; assert_eq!(result.unwrap(), review_ops_response); let result = client .pulls(OWNER, REPO) - .pull_number(PULL_NUMBER) - .reviews() - .review(REVIEW_ID) + .pr_review_actions(PULL_NUMBER, REVIEW_ID) .dismiss("test") .await; assert_eq!(result.unwrap(), review_ops_response); let result = client .pulls(OWNER, REPO) - .pull_number(PULL_NUMBER) - .reviews() - .review(REVIEW_ID) + .pr_review_actions(PULL_NUMBER, REVIEW_ID) .list_comments() .per_page(15) .send() @@ -146,9 +134,7 @@ async fn should_work_with_specific_review() { let result = client .pulls(OWNER, REPO) - .pull_number(PULL_NUMBER) - .comment(COMMENT_ID.into()) - .reply("test") + .reply_to_comment(PULL_NUMBER, COMMENT_ID.into(), "test") .await; assert_eq!(result.unwrap(), pr_comment_response); } diff --git a/tests/user_blocks_tests.rs b/tests/user_blocks_tests.rs index e15ea2ec..893e0300 100644 --- a/tests/user_blocks_tests.rs +++ b/tests/user_blocks_tests.rs @@ -115,5 +115,5 @@ async fn should_respond_user_unblocked() { .await; let client = setup_octocrab(&mock_server.uri()); let result = client.users("some-user").unblock_user(NOT_BLOCKED).await; - assert!(!result.is_ok()); + assert!(result.is_err()); }