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

refactor(azure-integration-resource): code refactor of azure integration resource to improve code readability #2773

Merged
merged 4 commits into from
Nov 11, 2024

Conversation

Aashirwadjain
Copy link
Collaborator

@Aashirwadjain Aashirwadjain commented Nov 11, 2024

Description

  • Refactored code to reduce code redundancy of schema attributes in the schema of the Azure Integrations Terraform
    resource.
  • Created wrapper function to replace repeated usages of d.GetOk()

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

Please delete options that are not relevant.

  • My commit message follows conventional commits
  • My code is formatted to Go standards
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes. Go here for instructions on running tests locally.

How to test this change?

  • Create the Azure Link Account
  • Update the Azure Link Account
  • Destroy the Azure Link Account

@Aashirwadjain Aashirwadjain changed the title refactor(azure-link-account): code refactor of azure link account resource to improve readability refactor(azure-link-account): code refactor of azure link account resource to improve code readability Nov 11, 2024
@pranav-new-relic
Copy link
Member

pranav-new-relic commented Nov 11, 2024

@Aashirwadjain everything looks good, a final bunch of things to address -

  • If you see the integration tests in resource_newrelic_cloud_azure_integrations_test.go and compare them with the ones you find in the integration tests results in the CI (i.e, the integration tests tasks you see in the PR), you'll find that one of the tests is being skipped. We had to skip that for quite some time since we had incorrect Azure credentials, but we now have them - we'd recently updated them in the environment of this GitHub repo, so we should be able to unskip it now. Can you find how to unskip it and push the change to do so, so we make sure that test runs too? 😉
  • We recently released a new version of newrelic-client-go, a dependency used by the provider to perform API calls to RESTv2 and NerdGraph, which is actually what helps manage resources in Terraform. Can you find the latest version of this, and update it in the dependencies of this repo? This would help us propagate changes of the latest client into Terraform, the moment we merge this PR.

Hope you can find these and get them done 👍

@Aashirwadjain
Copy link
Collaborator Author

@pranav-new-relic, Thanks for suggestion. I have addressed the issues that you suggested as below:

  • Integration tests in resource_newrelic_cloud_azure_integrations_test.go are unskipped and passing.
  • Updated newrelic-client-go to v2.51.3 in go.mod file and make compile is working successfully.

@pranav-new-relic
Copy link
Member

cool! Let's wait for the tests to run, and then we should be good to go.

@Aashirwadjain Aashirwadjain changed the title refactor(azure-link-account): code refactor of azure link account resource to improve code readability refactor(azure-integration-resource): code refactor of azure integration resource to improve code readability Nov 11, 2024
@pranav-new-relic pranav-new-relic merged commit 63c1e55 into main Nov 11, 2024
11 of 12 checks passed
@pranav-new-relic pranav-new-relic deleted the azure-link-account-code-refactor branch November 11, 2024 15:20
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