Refactor issue parsing and add GraphQL helper#18
Merged
Conversation
Contributor
Reviewer's GuideThis PR refactors GraphQL interactions by introducing a dedicated GraphQLClient with a generic run_query helper, consolidates pull request and issue reference parsing through a ResourceType-driven parse_reference routine, and enhances issue display with termimad-based markdown rendering. Sequence Diagram for
|
| Change | Details | Files |
|---|---|---|
| Introduce GraphQLClient and run_query helper |
|
src/main.rs |
| Unify PR and issue reference parsing |
|
src/main.rs |
| Enhance issue rendering with termimad |
|
src/main.rs |
| Update tests for parsing changes |
|
src/main.rs |
Tips and commands
Interacting with Sourcery
- Trigger a new review: Comment
@sourcery-ai reviewon the pull request. - Continue discussions: Reply directly to Sourcery's review comments.
- Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with@sourcery-ai issueto create an issue from it. - Generate a pull request title: Write
@sourcery-aianywhere in the pull
request title to generate a title at any time. You can also comment
@sourcery-ai titleon the pull request to (re-)generate the title at any time. - Generate a pull request summary: Write
@sourcery-ai summaryanywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment@sourcery-ai summaryon the pull request to
(re-)generate the summary at any time. - Generate reviewer's guide: Comment
@sourcery-ai guideon the pull
request to (re-)generate the reviewer's guide at any time. - Resolve all Sourcery comments: Comment
@sourcery-ai resolveon the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore. - Dismiss all Sourcery reviews: Comment
@sourcery-ai dismisson the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment
@sourcery-ai reviewto trigger a new review!
Customizing Your Experience
Access your dashboard to:
- Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others. - Change the review language.
- Add, remove or edit custom review instructions.
- Adjust other review settings.
Getting Help
- Contact our support team for questions or feedback.
- Visit our documentation for detailed guides and information.
- Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.
|
Warning Rate limit exceeded@leynos has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 1 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughA generic asynchronous function was introduced to handle GraphQL requests and responses, consolidating repeated logic. Reference parsing for pull requests and issues was unified via a new generic function. Existing functions were refactored to use these abstractions. Output formatting and unit tests were improved, and a GitHub GraphQL endpoint constant was added. Changes
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes - here's some feedback:
- The fallback parsing logic for plain numeric inputs was removed from
parse_reference, so passing just an issue/PR number now fails—reintroduce that branch or re-add it inparse_pr_reference/parse_issue_reference. - Consider parameterizing the GraphQL endpoint in
run_queryinstead of hardcoding the URL to improve flexibility and testability. - To avoid cloning the entire header map on each request, either take
headersby value or adjustrun_queryto borrow it without cloning.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The fallback parsing logic for plain numeric inputs was removed from `parse_reference`, so passing just an issue/PR number now fails—reintroduce that branch or re-add it in `parse_pr_reference`/`parse_issue_reference`.
- Consider parameterizing the GraphQL endpoint in `run_query` instead of hardcoding the URL to improve flexibility and testability.
- To avoid cloning the entire header map on each request, either take `headers` by value or adjust `run_query` to borrow it without cloning.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Owner
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes - here's some feedback:
- Consider replacing the raw &str resource_type parameter in parse_reference with a dedicated enum to catch typos at compile time and improve readability.
- Since every run_query call uses the same GitHub endpoint and headers, you could wrap the client + headers into a small GraphQLClient struct that defaults to the GitHub URL to simplify invocation.
- The #[allow(dead_code)] on the Unimplemented error variant can likely be removed once the refactor is complete or if that variant is no longer needed.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider replacing the raw &str resource_type parameter in parse_reference with a dedicated enum to catch typos at compile time and improve readability.
- Since every run_query call uses the same GitHub endpoint and headers, you could wrap the client + headers into a small GraphQLClient struct that defaults to the GitHub URL to simplify invocation.
- The #[allow(dead_code)] on the Unimplemented error variant can likely be removed once the refactor is complete or if that variant is no longer needed.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/main.rs (1)
630-638: 🛠️ Refactor suggestion
parse_referencemisses “pulls” URLs & other variantsGitHub sometimes serves PR URLs as
https://github.com/owner/repo/pulls/42(note the s).
The current checksegments[2] == "pull"therefore rejects a valid reference.- if segments.len() >= 4 && segments[2] == resource_type { + if segments.len() >= 4 + && (segments[2] == resource_type + || (resource_type == "pull" && segments[2] == "pulls")) + {Optionally extend the “issues” branch to tolerate the singular “issue” as well.
🧹 Nitpick comments (1)
src/main.rs (1)
145-171:run_querycan attach headers more idiomatically (and a tad faster)Iterating and inserting one-by-one is unnecessary;
RequestBuilder::headerstakes a wholeHeaderMap:- let mut req = client.post(endpoint); - for (k, v) in headers { - req = req.header(k, v); - } - let resp: GraphQlResponse<T> = req + let resp: GraphQlResponse<T> = client + .post(endpoint) + .headers(headers.clone()) .json(&json!({ "query": query, "variables": variables })) .send() .await? .json() .await?;Keeps the code shorter and avoids a small per-header allocation.
Owner
Author
|
@coderabbitai new github issue |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@leynos I'll create a GitHub issue for this parsing improvement. Let me set that up for you. 🏁 Script executed: Length of output: 1043 Perfect! I've successfully created GitHub issue #19 for the URL parsing improvement: #19 The issue includes:
The issue is now tracked and ready for implementation in a future PR. |
Owner
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes - here's some feedback:
- Rename ResourceType::Pull to ResourceType::PullRequest for clearer alignment with GitHub terminology and to avoid ambiguity.
- Enhance parse_reference error reporting to distinguish between an unrecognized URL format and a mismatched resource type, making it easier to diagnose parsing failures.
- Include contextual information (e.g., query name or variables) in GraphQLClient.run_query error handling so failed requests can be debugged more easily.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Rename ResourceType::Pull to ResourceType::PullRequest for clearer alignment with GitHub terminology and to avoid ambiguity.
- Enhance parse_reference error reporting to distinguish between an unrecognized URL format and a mismatched resource type, making it easier to diagnose parsing failures.
- Include contextual information (e.g., query name or variables) in GraphQLClient.run_query error handling so failed requests can be debugged more easily.
## Individual Comments
### Comment 1
<location> `src/main.rs:176` </location>
<code_context>
+ }
+ }
+
+ async fn run_query<V, T>(&self, query: &str, variables: V) -> Result<T, VkError>
+ where
+ V: serde::Serialize,
</code_context>
<issue_to_address>
Use DeserializeOwned for T bound in run_query
This change simplifies lifetime management and guarantees that T is fully owned.
Suggested implementation:
```rust
async fn run_query<V, T>(&self, query: &str, variables: V) -> Result<T, VkError>
where
V: serde::Serialize,
T: serde::de::DeserializeOwned,
```
```rust
use serde::de::DeserializeOwned;
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| } | ||
| } | ||
|
|
||
| async fn run_query<V, T>(&self, query: &str, variables: V) -> Result<T, VkError> |
Contributor
There was a problem hiding this comment.
suggestion: Use DeserializeOwned for T bound in run_query
This change simplifies lifetime management and guarantees that T is fully owned.
Suggested implementation:
async fn run_query<V, T>(&self, query: &str, variables: V) -> Result<T, VkError>
where
V: serde::Serialize,
T: serde::de::DeserializeOwned,use serde::de::DeserializeOwned;
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
run_queryhelper for GraphQL requeststermimadTesting
cargo clippy -- -D warningscargo testhttps://chatgpt.com/codex/tasks/task_e_684e2310e92483228e02d368d449cda9
Summary by Sourcery
Introduce a reusable GraphQL client, unify issue/PR parsing, and enhance output formatting
New Features:
Enhancements:
Tests:
Summary by CodeRabbit