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

Add support for destroy_data and update_data to be sent #182

Merged
merged 6 commits into from
Nov 2, 2022

Conversation

jgrumboe
Copy link
Contributor

@jgrumboe jgrumboe commented Jul 7, 2022

This PR is inspired by #177 and it adds one feature:

  • destroy_data => Allows specifying a 'data/body' to send along with destroy/delete requests.

It could solve #174 I think.

@jgrumboe
Copy link
Contributor Author

jgrumboe commented Jul 7, 2022

@DRuggeri Is this PR ok? Is the test sufficient?

If it's fine I could also "extend" this PR to solve #124 and add update_data in the same way.

@m-garrido
Copy link

It would be awesome to have the same feature for update and delete.

@jgrumboe
Copy link
Contributor Author

jgrumboe commented Aug 16, 2022

@m-garrido @melt-melt
I also added support for update_data.
@DRuggeri can you have a look at this?

@jgrumboe jgrumboe changed the title Add support for destroy_data to be sent Add support for destroy_data and update_data to be sent Aug 16, 2022
@jgrumboe
Copy link
Contributor Author

@DRuggeri I needed to update the GH release action also following this comment hashicorp/ghaction-import-gpg#11 (comment)
Otherwise I wasn't able to publish the provider interim to my personal TF registry account.

I can remove the commit before merging.

@jgrumboe
Copy link
Contributor Author

jgrumboe commented Aug 16, 2022

@m-garrido @melt-melt
I published my changes interim to TF registry under https://registry.terraform.io/providers/jgrumboe/restapi/1.18.0-dev
You could try out update_data and destroy_data for tests, would like to remove the provider from my account in the TF registry after this PR is merged.

Update: This PR is already merged, please use the official version >= 1.18.0!

Copy link

@arun-a-nayagam arun-a-nayagam left a comment

Choose a reason for hiding this comment

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

I have tested the update_data changes in an actual project and can confirm that it works fine.
Refer to [https://github.com//issues/190]

@jgrumboe
Copy link
Contributor Author

@DRuggeri can you help me here to get that merged? If something is missing like tests or docs, just tell me. Thanks in advance 🙏

@jgrumboe
Copy link
Contributor Author

@douernesto @Martina-May
Could you probably help in getting this PR merged? Unfortunately there's no feedback yet from @DRuggeri 😞

@Sh1ftry
Copy link

Sh1ftry commented Oct 26, 2022

Hello, I am also waiting for this feature. Would be nice to have it released soon.

Copy link
Member

@DRuggeri DRuggeri left a comment

Choose a reason for hiding this comment

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

This looks good, though I am concerned about some of the changes to the release workflow. Can you please address those?

.github/workflows/release.yaml Outdated Show resolved Hide resolved
.github/workflows/release.yaml Show resolved Hide resolved
.github/workflows/release.yaml Outdated Show resolved Hide resolved
docs/resources/object.md Outdated Show resolved Hide resolved
docs/resources/object.md Outdated Show resolved Hide resolved
restapi/api_object.go Outdated Show resolved Hide resolved
restapi/api_object_test.go Show resolved Hide resolved
@DRuggeri
Copy link
Member

DRuggeri commented Nov 2, 2022

Many apologies for the delay responding to this PR. This seems to make a lot of sense and I've added a quick few notes. If you can address those, we can go ahead and merge and cut a new release with this!

@jgrumboe
Copy link
Contributor Author

jgrumboe commented Nov 2, 2022

@DRuggeri Thanks for the feedback. I addressed it.

@DRuggeri DRuggeri merged commit 7616c02 into Mastercard:master Nov 2, 2022
@jgrumboe jgrumboe deleted the destroy-data branch November 2, 2022 21:19
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.

5 participants