-
Notifications
You must be signed in to change notification settings - Fork 73
Transfers API: method signature with required query parameters #1515
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
Conversation
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.
Summary of Changes
Hello @gcatanese, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request addresses a critical issue in API method signature generation by reordering parameters to place required query parameters first, which was causing errors. This change necessitates regenerating the Transfers
API services and introduces breaking changes to the getAllTransactions
and getAllTransfers
methods, making date range parameters mandatory. Additionally, a new executionDate
attribute is added to key transfer-related models, and general serialization logic is refined for improved flexibility.
Highlights
- API Method Signature Generation Fix: The core change involves updating the code generation templates (
api.mustache
) to correctly order method parameters, ensuring that all required query parameters are placed before optional ones. This resolves an issue where incorrect parameter order could lead to errors. - Breaking API Changes Introduced: The
TransactionsApi.getAllTransactions
andTransfersApi.getAllTransfers
methods now havecreatedSince
andcreatedUntil
as mandatory parameters, and their position in the method signature has been moved to the beginning. This is a breaking change for consumers of these APIs. - New
ExecutionDate
Model and Integration: A newExecutionDate
data model has been introduced, definingdate
andtimezone
properties. This new attribute has been integrated into theTransfer
,TransferData
, andTransferInfo
models, enhancing their data structure. - Updated Serialization Logic: The
ObjectSerializer.serialize
andObjectSerializer.deserialize
methods have been modified to make theformat
parameter optional, simplifying their usage across the codebase. - Unit Test Updates: Existing unit tests for
TransactionsApi.getAllTransactions
were updated, and new tests were added forTransfersApi.getTransfer
andTransfersApi.getAllTransfers
to validate the new method signatures and required parameter handling.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
Code Review
This pull request correctly updates the method signatures for the Transfers
and Transactions
APIs to place required query parameters (createdSince
, createdUntil
) before optional ones, addressing a potential issue with parameter order. It also includes the addition of the executionDate
attribute to relevant models and updates the ObjectSerializer
accordingly. The changes in the templates reflect the desired signature generation logic. Unit tests have been updated to match the new signatures and cover the new getTransfer
endpoint.
|
When generating the method signatures the required query parameters must be before all optional parameters, to avoid causing an error (invalid order of parameters).
This PR performs the following:
Transfers
APIexecutionDate
attribute inTransfer
,TransferData
andTransferInfo
Breaking Changes 🛠
This change introduces a breaking change because the previous version of the library marked (incorrectly) all query parameters as optional.
TransactionsApi.getAllTransactions
becomes
createdSince
ancreatedUntil
are the first 2 parametersTransfersApi.getAllTransfers
becomes
createdSince
ancreatedUntil
are the first 2 parameters