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

azuread_application: Added support for Required Resource Access #23

Merged
merged 6 commits into from
Jan 28, 2019

Conversation

gerardaus
Copy link
Contributor

@gerardaus gerardaus commented Jan 22, 2019

In order to support the "Required Resource Access Type", the azure sdk needed to be upgraded.

(fixes #9)

@katbyte
Copy link
Collaborator

katbyte commented Jan 22, 2019

Hi @gerardaus,

Thank you for the contribution! With respect to SDK upgrades we usually do them as a separate PR from resource changes, as well as keep all packages at the same version. I have opened #24 & #25 that will update the sdk & autorest packages, once merged you should be able to merge master and only the resource changes will remain here.

@gerardaus
Copy link
Contributor Author

Hi @gerardaus,

Thank you for the contribution! With respect to SDK upgrades we usually do them as a separate PR from resource changes, as well as keep all packages at the same version. I have opened #24 & #25 that will update the sdk & autorest packages, once merged you should be able to merge master and only the resource changes will remain here.

@katbyte done

@ghost ghost removed the waiting-response label Jan 23, 2019
@gerardaus gerardaus force-pushed the master branch 2 times, most recently from 2900ad3 to f85c696 Compare January 23, 2019 01:39
Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

Thanks @gerardaus,

I have looked over the PR and left some comments inline. In addition could we also update the data source with these new properties?

azuread/resource_application.go Outdated Show resolved Hide resolved
azuread/resource_application.go Show resolved Hide resolved
azuread/resource_application.go Show resolved Hide resolved
azuread/resource_application.go Show resolved Hide resolved
azuread/resource_application.go Outdated Show resolved Hide resolved
azuread/resource_application_test.go Outdated Show resolved Hide resolved
website/docs/r/application.html.markdown Outdated Show resolved Hide resolved
@gerardaus
Copy link
Contributor Author

@katbyte good for another review

@ghost ghost removed the waiting-response label Jan 25, 2019
Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

hey @gerardaus

Thanks for pushing those changes - I've taken a look through and left some minor comments in-line but this is otherwise looking good; if we can fix those up then this should be good to merge 👍

Thanks!

azuread/data_application.go Outdated Show resolved Hide resolved
azuread/data_application.go Outdated Show resolved Hide resolved
azuread/data_application.go Outdated Show resolved Hide resolved
azuread/data_application.go Outdated Show resolved Hide resolved
azuread/data_application.go Outdated Show resolved Hide resolved
website/docs/d/application.html.markdown Outdated Show resolved Hide resolved
website/docs/d/application.html.markdown Outdated Show resolved Hide resolved
website/docs/r/application.html.markdown Outdated Show resolved Hide resolved
website/docs/r/application.html.markdown Outdated Show resolved Hide resolved
website/docs/r/application.html.markdown Outdated Show resolved Hide resolved
@katbyte katbyte changed the title Added support for Required Resource Access Type within provider azuread_application; upgrade to azure-sdk-for-go graphrbac v24.1.0 Added support for Required Resource Access Type within provider azuread_application Jan 25, 2019
@katbyte katbyte changed the title Added support for Required Resource Access Type within provider azuread_application azuread_application: Added support for Required Resource Access Jan 25, 2019
@gerardaus
Copy link
Contributor Author

@tombuildsstuff ready to roll. Assuming its good to merge, do you want me to squash those commits?

@ghost ghost removed the waiting-response label Jan 26, 2019
@katbyte
Copy link
Collaborator

katbyte commented Jan 26, 2019

@gerardaus, we can easily squash them via github's merge button 🙂 i'll give it a review shortly

@katbyte
Copy link
Collaborator

katbyte commented Jan 28, 2019

Thanks for the updates @gerardaus, LGTM now 🚀

@katbyte katbyte merged commit a25cfc9 into hashicorp:master Jan 28, 2019
katbyte added a commit that referenced this pull request Jan 28, 2019
@ghost
Copy link

ghost commented Mar 5, 2019

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

azuread_application: support for required-resource-accesses?
3 participants