Skip to content
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

Post Go-API import modifications #284

Merged
merged 4 commits into from
Jun 10, 2024
Merged

Conversation

dentarg
Copy link
Member

@dentarg dentarg commented Jun 7, 2024

WHY are these changes introduced?

#282 imported https://github.com/84codes/go-api with history

#273 makes some changes to the library code, the commits are cherry-picked from that branch

WHAT is this pull request doing?

  • Directly use go-api client library in the provider
  • Remove the dependancy to separated go-api client library
  • Clean up code and logging.
  • More coherent code structure
    • Declare variable within var()
    • Make request
    • Switch statement to handle response

HOW can this pull request be tested?

  • Test with Go-VCR test fixtures and subset of new tests

tbroden84 added 3 commits June 7, 2024 11:51
Make the client library more coherent and follow the same structure

- Declare variable within var()
- Make request
- Switch statement to handle response
Updates map[string]interface{} and similar to instead use the any alias.
@dentarg
Copy link
Member Author

dentarg commented Jun 7, 2024

Oh, the changes to cloudamqp/ and go.mod and go.sum is missing. They're part of 1727c2b that also imported api/ files, so I never attempted to cherry-pick that.

Removes the dependency to the separated go-api client library as it is
now included directly in the provider.

Co-authored-by: Tobias Brodén <[email protected]>
@dentarg dentarg marked this pull request as ready for review June 7, 2024 10:18
@dentarg
Copy link
Member Author

dentarg commented Jun 7, 2024

I extracted those changes by hand and added them here. CI is green 🥳

Please test this as I haven't @tbroden84 :)

@tbroden84
Copy link
Contributor

@dentarg GJ! Will do.

Copy link
Contributor

@tbroden84 tbroden84 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried a subset of the tests with real requests together with some manual ones. Looking good to me.

List of the subset tests used.
TestAccAlarm_Basic
TestAccInstance_Basic
TestAccExtraDiskSize_AWS_Basic
TestAccIntegrationLog_Basic
TestAccRabbitMqConfiguration_Basic
TestAccFirewall_Basic
TestAccVpcConnect_AWS_Basic
TestAccVpcConnect_GCP_Basic
TestAccVpcConnect_Azure_Basic

@tbroden84 tbroden84 merged commit 3135a87 into main Jun 10, 2024
2 checks passed
@tbroden84 tbroden84 deleted the go-api/post-import-modifications branch June 10, 2024 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants