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.

Sign up for GitHub

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

Jump to bottom

azurerm_application_gateway: Adding upload limit #2666

Merged
merged 4 commits into from
Jan 16, 2019
Merged

azurerm_application_gateway: Adding upload limit #2666

merged 4 commits into from
Jan 16, 2019

Conversation

ghostinthewires
Copy link
Contributor

This PR introduces the ability to set the file upload limit in the Azure Application Gateway WAF (requested in #2409)

tombuildsstuff
tombuildsstuff suggested changes Jan 14, 2019
View reviewed changes
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 @ghostinthewires

Thanks for this PR :)

I've taken a look through and this mostly LGTM - the main question I've got is around the naming of the field as I think it'd be better to more closely mirror the API/SDK naming - but if we can fix the minor comments up we should be able to run the tests and get this merged 👍

Thanks!

azurerm/resource_arm_application_gateway.go Outdated Show resolved Hide resolved
azurerm/resource_arm_application_gateway_test.go Outdated Show resolved Hide resolved
azurerm/resource_arm_application_gateway.go Outdated Show resolved Hide resolved
website/docs/r/application_gateway.html.markdown Outdated Show resolved Hide resolved
azurerm/resource_arm_application_gateway.go Outdated Show resolved Hide resolved
metacpp
metacpp reviewed Jan 15, 2019
View reviewed changes
Copy link
Contributor

@metacpp metacpp left a comment

Choose a reason for hiding this comment

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

Thanks for the great contributions, most LGTM but please address comments left by @tombuildsstuff and me.

website/docs/r/application_gateway.html.markdown Show resolved Hide resolved
@ghostinthewires
Copy link
Contributor Author

Hi,

I've implemented the changes as suggested. Hopefully should be good to go now!

Just let me know if there is anything else

@ghost ghost removed the waiting-response label Jan 15, 2019
metacpp
metacpp approved these changes Jan 15, 2019
View reviewed changes
Copy link
Contributor

@metacpp metacpp left a comment

Choose a reason for hiding this comment

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

@ghostinthewires Thanks for the updates, LGTM now.

katbyte
katbyte approved these changes Jan 16, 2019
View reviewed changes
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 @ghostinthewires, LGTM now 👍

@katbyte katbyte added this to the 2.0.0 milestone Jan 16, 2019
@katbyte katbyte merged commit 964ddef into hashicorp:master Jan 16, 2019
katbyte added a commit that referenced this pull request Jan 16, 2019
@katbyte
Update CHANGELOG.md to include #2666
55068f0
@ghostinthewires ghostinthewires deleted the adding-upload-limit-appgw-waf branch January 16, 2019 08:51
@tombuildsstuff tombuildsstuff modified the milestones: 2.0.0, 1.22.0 Jan 21, 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.
Reviewers

@tombuildsstuff tombuildsstuff tombuildsstuff requested changes

@metacpp metacpp metacpp approved these changes

@katbyte katbyte katbyte approved these changes

Assignees
No one assigned
Labels
documentation enhancement service/application-gateway size/L
Projects
None yet
Milestone
v1.22.0
Development

Successfully merging this pull request may close these issues.
4 participants
@ghostinthewires @tombuildsstuff @katbyte @metacpp