search

cerbo / aws-waf-security-automation

Amazon WAF Security Automation deployment (modular with Terraform)
Apache License 2.0
73 stars 27 forks source link

Patch 1 #10

Closed mikerj1 closed 5 years ago

mikerj1 commented 5 years ago

random string sometimes contains uppercase characters, which are not valid for S3 bucket names

mikerj1 commented 5 years ago

Changed pull request to v2 branch

ventz commented 5 years ago

@mikerj1 Incredible PR! Great job on the changes and fixes.

Let me go through it, but so far it looks good!

ventz commented 5 years ago

@mikerj1 Could you break this up into "upgrades" vs "improvements".

That is, "upgrades" being the new node_modules and 3rd-party functions that are replacing/upgrading Amazon's provided code

vs

"Improvements" -- all of the new improvements you are introducing.

Just so that it's easier/a bit more sane to go through.

There are 322 changes files currently...

mikerj1 commented 5 years ago

I think the fact that I initially chose the wrong branch is making this look far more impressive than it is. It is a one line change. May be better to reject and I can submit properly again.

ventz commented 5 years ago

@mikerj1 Ok, closing it so you can resubmit.