LEM v4 #1197
LEM v4 #1197
Conversation
Summary of ChangesHello @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 updates the client library to align with version 4 of the Legal Entity Management (LEM) API. The changes involve a comprehensive overhaul of existing models, including the removal of deprecated fields, the introduction of new data structures and fields to support expanded functionalities, and updates to API endpoints. This ensures compatibility with the latest LEM API specification and provides access to new features for managing legal entities. Highlights
Using Gemini Code AssistThe 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 by creating a comment using either
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 Limitations & Feedback Gemini Code Assist 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Legal Entity Management API models and services to v4. The changes, including breaking changes, new fields, and enum values, are mostly well-implemented and align with the PR description. My review focuses on a few areas for improvement, primarily around ensuring API contracts are strictly enforced in the code (especially for required fields) and documenting all source-breaking changes, such as the re-ordering of enum integer values.
| /// <summary> | ||
| /// Enum Issuing for value: issuing | ||
| /// </summary> | ||
| [EnumMember(Value = "issuing")] | ||
| Issuing = 2, | ||
|
|
||
| /// <summary> | ||
| /// Enum Banking for value: banking | ||
| /// </summary> | ||
| [EnumMember(Value = "banking")] | ||
| Banking = 2 | ||
| Banking = 3 |
There was a problem hiding this comment.
The underlying integer value for Banking in ServiceEnum has changed from 2 to 3 due to the insertion of the new Issuing member. This is a source-breaking change for consumers who might rely on the integer values of enums. To maintain backward compatibility of integer values, it's recommended to append new enum members at the end. Please consider keeping Banking = 2 and assigning Issuing = 3.
/// <summary>
/// Enum Banking for value: banking
/// </summary>
[EnumMember(Value = "banking")]
Banking = 2,
/// <summary>
/// Enum Issuing for value: issuing
/// </summary>
[EnumMember(Value = "issuing")]
Issuing = 3| /// <summary> | ||
| /// Enum Issuing for value: issuing | ||
| /// </summary> | ||
| [EnumMember(Value = "issuing")] | ||
| Issuing = 2, | ||
|
|
||
| /// <summary> | ||
| /// Enum Banking for value: banking | ||
| /// </summary> | ||
| [EnumMember(Value = "banking")] | ||
| Banking = 2 | ||
| Banking = 3 |
There was a problem hiding this comment.
The underlying integer value for Banking in ServiceEnum has changed from 2 to 3 due to the insertion of the new Issuing member. This is a source-breaking change for consumers who might rely on the integer values of enums. To maintain backward compatibility, it's recommended to append new enum members at the end. Please consider keeping Banking = 2 and assigning Issuing = 3.
/// <summary>
/// Enum Banking for value: banking
/// </summary>
[EnumMember(Value = "banking")]
Banking = 2,
/// <summary>
/// Enum Issuing for value: issuing
/// </summary>
[EnumMember(Value = "issuing")]
Issuing = 3| /// <summary> | ||
| /// Enum GbSortCode for value: gbSortCode | ||
| /// Enum AuBsbCode for value: auBsbCode | ||
| /// </summary> | ||
| [EnumMember(Value = "gbSortCode")] | ||
| GbSortCode = 1, | ||
| [EnumMember(Value = "auBsbCode")] | ||
| AuBsbCode = 1, | ||
|
|
||
| /// <summary> | ||
| /// Enum UsRoutingNumber for value: usRoutingNumber | ||
| /// Enum CaRoutingNumber for value: caRoutingNumber | ||
| /// </summary> | ||
| [EnumMember(Value = "usRoutingNumber")] | ||
| UsRoutingNumber = 2, | ||
| [EnumMember(Value = "caRoutingNumber")] | ||
| CaRoutingNumber = 2, | ||
|
|
||
| /// <summary> | ||
| /// Enum AuBsbCode for value: auBsbCode | ||
| /// Enum GbSortCode for value: gbSortCode | ||
| /// </summary> | ||
| [EnumMember(Value = "auBsbCode")] | ||
| AuBsbCode = 3, | ||
| [EnumMember(Value = "gbSortCode")] | ||
| GbSortCode = 3, | ||
|
|
||
| /// <summary> | ||
| /// Enum CaRoutingNumber for value: caRoutingNumber | ||
| /// Enum UsRoutingNumber for value: usRoutingNumber | ||
| /// </summary> | ||
| [EnumMember(Value = "caRoutingNumber")] | ||
| CaRoutingNumber = 4 | ||
| [EnumMember(Value = "usRoutingNumber")] | ||
| UsRoutingNumber = 4 | ||
|
|
||
| } |
There was a problem hiding this comment.
The underlying integer values of TypeEnum have been reordered, which is a source-breaking change for consumers who might cast these enum values to integers. For example, GbSortCode was 1 and is now 3. While the library's README warns against relying on enum integer values, this change should be explicitly documented in the pull request description as a breaking change to alert consumers.
1 participant
In this branch LEM services and models have been updated to support LEM v4.
See our documentation:
Note: generation of comments (attribute description) needs to be improved, to do in a different ticket.
Changes
Breaking Changes 🛠
BusinessLine: removecapabilitySourceOfFunds: removeacquiringBusinessLineIdSourceOfFunds: makeadyenProcessedFundsrequiredOther Changes 💎
Individual: addsupportOrganization: addsupport,doingBusinessAsAbsent,registrationNumberAbsentSoleProprietorship: adddoingBusinessAsAbsent,registrationNumberAbsentTrust: adddoingBusinessAsAbsentUnincorporatedPartnership: adddoingBusinessAsAbsentServiceEnum: add new value"issuing"SourceOfFunds: add several new attributes (amount,assetMonthsHeld, etc..)SourceOfFunds: add array ofFinancierSourceOfFunds. TypeEnum: add several new values ("employment","donations", etc..)TaxInformation: addnumberAbsentDocument: addproofOfDirector