-
Notifications
You must be signed in to change notification settings - Fork 1
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
Generate Client using OpenAPI spec tool, Update Setup Guide and Quickstart #2
Open
sajitha-tj
wants to merge
72
commits into
ballerina-platform:main
Choose a base branch
from
sajitha-tj:main
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains 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
tharindu-nw
reviewed
Jan 3, 2025
All of the API endpoints had the common prefix `/crm/v3/lists`. Those paths are modified to remove this prefix from the endpoints, and included in the base url. This centralizes the versioning to the base url and simplifies the API paths, making them more readable.
As mentioned in the sanitations.md file, few feilds needed changes inorder to generate the connector client correctly.
Previously used resource methods caused a "resource method is ambiguous" error for the endpoint "/crm/v3/lists/folders". Inorder to resolve this error, Ballerina connector was regenerated with remote methods and this fixed the issue. Additionally, all test cases were updated accordingly with the new generated method names, ensuring compatibility with the updated client methods.
README.md and the module.md files were updated with necessary details about the package. Also did some minor changes on other documentations as well.
A mock service which simulates some of HubSpot CRM API responses was added to allow local testing without hitting the actual HubSpot server. Tests in the tests.bal file were grouped based on the ability to running them with mock server.
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.
Good job overall!
Co-authored-by: Thisaru Guruge <[email protected]>
Co-authored-by: Thisaru Guruge <[email protected]>
Few changes were added as suggested on the PR review. The function used to get the list of lead IDs was simplified to return a list of dummy IDs. In a real world scenario, a database fetch or any other method can be implemented to fetch the relevent IDs.
Since there are only a few endpoints at the mock server, some tests fail when they are executed at the build stage. This update prevents execution of these tests, when the mock server is used.
Previously the mock service started even with live tests. This change will prevent execution of mock server when the live hubspot server is used for testing.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Purpose
Initial setup of the ballerina connector
Examples
Checklist