-
Notifications
You must be signed in to change notification settings - Fork 3
Rename xdai to gnosis, mainnet to ethereum (where possible). #114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Viem only supports gnosis and mainnet as chain name, 'ethereum' is not a supported chain name. Fixes RequestNetwork#34
|
Hello @pclaesen, thank you for submitting your first pull request to the request-scan repository. We value your contribution and encourage you to review our contribution guidelines to ensure your submission meets our standards. Please note that every merged PR is automatically enrolled in our Best PR Initiative, offering a chance to win $500 each quarter. Our team is available via GitHub Discussions or Discord if you have any questions. Welcome aboard! |
WalkthroughUpdates blockchain chain identifiers throughout the codebase by replacing XDAI with GNOSIS, removing MAINNET from enums, adding ETHEREUM mappings, and refactoring GraphQL payment network field references across multiple query definitions to reflect new chain source mappings. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 🧪 Unit Test Generation v2 is now available!We have significantly improved our unit test generation capabilities. To enable: Add this to your reviews:
finishing_touches:
unit_tests:
enabled: trueTry it out by using the Have feedback? Share your thoughts on our Discord thread! Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Thank you for your submission! As you prepare for the review process, please ensure that your PR title, description, and any linked issues fully comply with our contribution guidelines. A clear explanation of your changes and their context will help expedite the review process. Every merged PR is automatically entered into our Best PR Initiative, offering a chance to win $500 every quarter. We appreciate your attention to detail and look forward to reviewing your contribution! |
Greptile OverviewGreptile SummaryRenamed chain identifiers from The changes update:
The actual chain values remain correct ( A minor style inconsistency exists in Confidence Score: 4.5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant App as Application
participant Consts as consts.ts
participant Queries as GraphQL Queries
participant GQL as GraphQL Client
participant Subgraph as The Graph Subgraph
participant Viem as Viem Public Clients
Note over Consts: CHAINS.ETHEREUM = "mainnet"<br/>CHAINS.GNOSIS = "gnosis"
Note over Consts: PAYMENT_CHAINS.ETHEREUM = "payment_ethereum"<br/>PAYMENT_CHAINS.GNOSIS = "payment_gnosis"
App->>Queries: fetchPayments()
Queries->>GQL: Execute PAYMENTS_QUERY
Note over Queries: Query uses payment_ethereum<br/>instead of payment_mainnet
GQL->>Subgraph: payment_ethereum { payments(...) }
GQL->>Subgraph: payment_gnosis { payments(...) }
Subgraph-->>GQL: Payment data
GQL-->>Queries: Formatted results
Queries-->>App: Payment[]
App->>Consts: Access PUBLIC_CLIENTS[CHAINS.ETHEREUM]
Consts->>Viem: createPublicClient({ chain: mainnet })
Note over Viem: Viem expects "mainnet" and "gnosis"<br/>as valid chain identifiers
Viem-->>Consts: Public client instance
Consts-->>App: Configured client
App->>Consts: Access CHAIN_SCAN_URLS[CHAINS.ETHEREUM]
Consts-->>App: "https://etherscan.io"
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 files reviewed, 2 comments
| payment_ethereum { | ||
| payments( | ||
| where: { reference: $reference } | ||
| orderBy: timestamp | ||
| orderDirection: desc | ||
| where: { reference: $reference } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent parameter ordering: where comes after orderBy in payment_ethereum block, while all other blocks have where first.
| payment_ethereum { | |
| payments( | |
| where: { reference: $reference } | |
| orderBy: timestamp | |
| orderDirection: desc | |
| where: { reference: $reference } | |
| payment_ethereum { | |
| payments( | |
| where: { reference: $reference } | |
| orderBy: timestamp | |
| orderDirection: desc |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/queries/request-payments.ts
Line: 59:63
Comment:
Inconsistent parameter ordering: `where` comes after `orderBy` in `payment_ethereum` block, while all other blocks have `where` first.
```suggestion
payment_ethereum {
payments(
where: { reference: $reference }
orderBy: timestamp
orderDirection: desc
```
How can I resolve this? If you propose a fix, please make it concise.| payment_arbitrum_one { | ||
| payments( | ||
| where: { reference: $reference } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent parameter ordering: where comes before orderBy in payment_arbitrum_one block, while the old payment_mainnet had it after. Consider keeping same order as other blocks.
| payment_arbitrum_one { | |
| payments( | |
| where: { reference: $reference } | |
| payment_arbitrum_one { | |
| payments( | |
| where: { reference: $reference } |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/queries/request-payments.ts
Line: 14:16
Comment:
Inconsistent parameter ordering: `where` comes before `orderBy` in `payment_arbitrum_one` block, while the old `payment_mainnet` had it after. Consider keeping same order as other blocks.
```suggestion
payment_arbitrum_one {
payments(
where: { reference: $reference }
```
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.
Viem only supports gnosis and mainnet as chain name, 'ethereum' is not a supported chain name. Fixes #34
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.