fix(tools): fix connector template generation #271
Conversation
WalkthroughThe pull request involves a systematic update across multiple client template files in the Changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (6)
🔇 Additional comments (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
tools/connector-template/template/client/external_accounts.gotpl (1)
6-6: Overall changes look good with one suggested improvement.The migration from httpwrapper to a dedicated metrics package is a good architectural decision that:
- Improves separation of concerns
- Makes the metrics handling more maintainable
- Standardizes the approach across different client operations
Consider documenting the list of valid operation names to ensure consistency across the codebase.
tools/connector-template/template/client/client.gotpl (2)
27-27: Remove unnecessary space in function signature.The space between the parentheses in the function signature is unnecessary.
-func New( /* TODO: fill config parameters */ ) *client { +func New(/* TODO: fill config parameters */) *client {
29-30: Enhance the TODO comment for transport options.The current TODO comment about transport options could be more descriptive to better guide implementers.
- // TODO: you can set an underlying http transport in metrics.TransportOpts for authentication for example + // TODO: Set authentication in metrics.TransportOpts.Transport if needed. + // Example: metrics.TransportOpts{Transport: &oauth2.Transport{...}}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
tools/connector-template/template/client/accounts.gotpl(1 hunks)tools/connector-template/template/client/balances.gotpl(1 hunks)tools/connector-template/template/client/client.gotpl(2 hunks)tools/connector-template/template/client/external_accounts.gotpl(1 hunks)tools/connector-template/template/client/payouts.gotpl(1 hunks)tools/connector-template/template/client/transactions.gotpl(1 hunks)tools/connector-template/template/client/transfers.gotpl(1 hunks)
🔇 Additional comments (6)
tools/connector-template/template/client/balances.gotpl (1)
6-6: LGTM! Verify operation name consistency.The changes correctly migrate from httpwrapper to metrics package. The operation name "list_account_balances" appears consistent with the function's purpose.
Let's verify the operation name consistency across the codebase:
Also applies to: 12-12
✅ Verification successful
Operation name "list_account_balances" is used consistently
The operation name is consistently used in both the connector template and moneycorp connector implementation for balance listing operations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other occurrences of this operation name to ensure consistency rg "list_account_balances"Length of output: 326
tools/connector-template/template/client/accounts.gotpl (1)
6-6: LGTM! Verify operation name consistency.The changes correctly migrate from httpwrapper to metrics package. The operation name "list_accounts" appears consistent with the function's purpose.
Let's verify the operation name consistency across the codebase:
Also applies to: 12-12
✅ Verification successful
Operation name "list_accounts" is consistently used across connectors
The operation name follows the established pattern used in other connector implementations (moneycorp, modulr, bankingcircle, etc.).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other occurrences of this operation name to ensure consistency rg "list_accounts"Length of output: 1250
tools/connector-template/template/client/transactions.gotpl (1)
6-6: LGTM! Verify operation name consistency.The changes correctly migrate from httpwrapper to metrics package. The operation name "list_transactions" appears consistent with the function's purpose.
Let's verify the operation name consistency across the codebase:
Also applies to: 12-12
✅ Verification successful
Operation name "list_transactions" is consistently used across connectors
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other occurrences of this operation name to ensure consistency rg "list_transactions"Length of output: 1346
tools/connector-template/template/client/payouts.gotpl (1)
6-6: LGTM! Import and context key changes are consistent.The changes correctly update the metrics context key source while maintaining the same functionality.
Also applies to: 14-14
tools/connector-template/template/client/transfers.gotpl (1)
6-6: LGTM! Import and context key changes are consistent.The changes correctly update the metrics context key source while maintaining the same functionality.
Also applies to: 14-14
tools/connector-template/template/client/client.gotpl (1)
7-7: LGTM! Metrics integration looks good.The addition of metrics transport while maintaining httpwrapper client functionality is well structured. The template variable
{{ .Connector }}is correctly used for metrics identification.Also applies to: 29-31
tools/connector-template/template/client/external_accounts.gotpl
Outdated
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #271 +/- ##
=======================================
Coverage 72.86% 72.86%
=======================================
Files 538 538
Lines 26986 26986
=======================================
Hits 19664 19664
Misses 6259 6259
Partials 1063 1063 ☔ View full report in Codecov by Sentry. |
paul-nicolas
force-pushed
the
fix/connector-template-generation
branch
from
dd76462 to
e73eb56
Compare
No description provided.