Skip to content
This repository has been archived by the owner on Nov 14, 2020. It is now read-only.

rabbitmq_queue: Set ForceNew on all attributes #53

Merged
merged 1 commit into from
Feb 21, 2020

Conversation

cyrilgdn
Copy link
Contributor

PR #38 added it on arguments attributes but actually we need it on all attributes
as a queue cannot be updated.

This fix #17

@aminueza As you seem to be quite motivated to work on this provider these days, I allow myself to ask for your help again for a little review :)

PR #38 added it on `arguments` attributes but actually we need it on all attributes
as a queue cannot be updated.

This fix #17
@ghost ghost added the size/S label Feb 12, 2020
@aminueza
Copy link
Contributor

aminueza commented Feb 13, 2020

@cyrilgdn the changes have been approved :) I have only some concerns regarding lint, format and sec of Golang code: what do you think in add some GitHub actions to check sec, format and lint? As I have done in my provider.
Those actions were made by me and work perfectly. I realized there are some errors about that.
I can make a PR for those changes if you approve :D

@cyrilgdn
Copy link
Contributor Author

@aminueza Thanks a lot for your review!

some concerns regarding lint, format and sec of Golang code

Did you see some errors?

what do you think in add some GitHub actions to check sec

I have mixed feeling about this. On one side I think it's a great idea to have this but I have 2 concerns:

  • I like to be able to run the same checks directly on my local machine (with a Makefile target or something like that), otherwise this means that I need to wait for the Github PR to see the errors I need to fix.
  • It seems that it will need to change all the d.Set because we are not handling the errors, so we'll have to do _ := d.Set which is just a lint-friendly code, and I don't see that in other providers. (but as we have not so many resources/attributes in this one.... I can consider it)

@cyrilgdn cyrilgdn merged commit c34b0a4 into master Feb 21, 2020
@cyrilgdn cyrilgdn deleted the queue-settings-forcenew branch February 21, 2020 16:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ForceNew True not working.
2 participants