Closed iamsumit closed 4 months ago
Attention: Patch coverage is 51.42857%
with 17 lines
in your changes missing coverage. Please review.
Project coverage is 38.40%. Comparing base (
3ff4ee2
) to head (bdec0ed
). Report is 66 commits behind head on main.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Hi @iamsumit and @coderkakarrot, thank you for submitting these additions to the New Relic Go Client. While all of the changes you've made are exactly on par with what one is supposed to do via Tutone, we've noticed a few discrepancies with the generated code (upon trying this myself), in response to which I had to make a couple of other changes to your code - just wanted to share these with you for context. This note shall also be useful for maintainers of the New Relic Go Client to take note of.
accountmanagement
package, alongside the new mutation you guys have added, changes have been made to the GetManagedAccounts()
function to take isCanceled
as an argument to the function, as this is now an optional variable used in the query. However, Tutone doesn't seem to be updating the query with this boolean variable, probably missing out on this because it is an optional variable.GetManagedAccounts()
function now has an additional argument which needs to be specified, which would result in accounts being filtered by isCanceled false or true, in order to not inflict breaking changes on dependent upstream services such as utilities managing account management in the New Relic Terraform Provider, we would need to convert isCanceled bool into isCanceled *bool, which also is currently unsupported by Tutone.Owing to the above, we've moved duplicated code of the GetManagedAccounts()
function with the above changes to a new file accountmanagement_api_.go
, while accountmanagement_api.go
would continue to have GetManagedAccounts()
not yet updated via Tutone, so we have this causing no probable impact and you guys are unblocked too, while we evaluate if this can be brought back into Tutone scope sometime in future. Hence, if you'd like to use isCanceled
with GetManagedAccounts()
, please use the version of this function in accountmanagement_api_.go
, GetManagedAccountsWithAdditionalArguments()
for time being.
Rest assured, the functions for the new mutations you've added one each in accountmanagement
and organization
are untouched and shall be merged as they are.
I shall proceed with merging this PR and releasing these changes; shall keep you guys posted if we find a way sometime to bring accountmanagement_api_.go
back into Tutone scope.
@iamsumit FYI: these changes have just been released with v2.41.0 of the New Relic Go Client 🎉
@pranav-new-relic Thank you, @pranav-new-relic for this.
We need these mutations for the project we're implementing this so adding it as PR. Thanks!