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

AWS WAF Regional #13676

Closed
wants to merge 3 commits into from
Closed

AWS WAF Regional #13676

wants to merge 3 commits into from

Conversation

yusukegoto
Copy link
Contributor

@yusukegoto yusukegoto commented Apr 15, 2017 •
edited
Loading

Resolving conflicts of #11263 coz original author is silent for weeks in spite of his great work.
Add implementations of change token consideration from #13656 .

@yusukegoto yusukegoto force-pushed the aws-wafregional branch 3 times, most recently from 7dcd11d to 56b0b63 Compare April 15, 2017 20:43
@yusukegoto yusukegoto changed the title [WIP] Aws wafregional Aws WAF Regional Apr 15, 2017
@yusukegoto yusukegoto changed the title Aws WAF Regional AWS WAF Regional Apr 15, 2017
@radeksimko
Copy link
Member

radeksimko commented Apr 16, 2017 •
edited
Loading

Hi @yusukegoto
and thanks @neildothunter for the initial work and you for the updates.

Would either of you mind submitting vendor updates (any changes in /vendor/*) in a separate PR, so we can reduce the LOC in the real PR and make it a bit easier to review?

I'm happy to merge that one pretty much straight away and let you rebase.

If you could then submit 1 resource at a time (with relevant docs + tests) that would be awesome as well.
Less LOC will be easier/faster to review. Most of us, maintainers prefer to review smaller PRs rather than bigger ones. 😃

@radeksimko radeksimko added the waiting-response An issue/pull request is waiting for a response from the community label Apr 16, 2017
@yusukegoto yusukegoto mentioned this pull request Apr 16, 2017
Merged
@yusukegoto
Copy link
Contributor Author

rebasing... 🏃

@yusukegoto
Copy link
Contributor Author

sorry, this is my best.

@yusukegoto
Copy link
Contributor Author

@radeksimko hi, should I create PRs by each resources???

@yusukegoto yusukegoto mentioned this pull request Apr 17, 2017
Closed
@yusukegoto
Copy link
Contributor Author

I think this PR will be closed after finising splits.

This was referenced Apr 17, 2017
Merged
Closed
Closed
Closed
Closed
Closed
Closed
@yusukegoto
Copy link
Contributor Author

yusukegoto commented Apr 17, 2017 •
edited
Loading

WIP ones will be rebased after merging dependencies.

@radeksimko
Copy link
Member

Thanks @yusukegoto ,
I really appreciate the effort you put into splitting this out 👍 👍
I will have a look at those PRs this week.

@radeksimko radeksimko removed the waiting-response An issue/pull request is waiting for a response from the community label Apr 18, 2017
@neildothunter
Copy link

@yusukegoto thanks for moving this forward (and sorry commitments stopped my further contribution when the PR got some focus). @radeksimko, would it be worth noting that resources should be submitted in individual PRs in the contribution guide?

@radeksimko radeksimko mentioned this pull request Apr 18, 2017
Merged
@radeksimko
Copy link
Member

@neildothunter No worries. Good idea about the update of contribution guide, see #13737

@yusukegoto do you mind if we close this PR in favour of the other ones?

@radeksimko radeksimko added the waiting-response An issue/pull request is waiting for a response from the community label Apr 18, 2017
@yusukegoto
Copy link
Contributor Author

@reedloden yes, please close this PR👍 .
@neildothunter hi, it's nice to see you!

@radeksimko radeksimko closed this Apr 18, 2017
@radeksimko radeksimko removed the waiting-response An issue/pull request is waiting for a response from the community label Apr 18, 2017
@ghost
Copy link

ghost commented Apr 13, 2020

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
provider/aws
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@yusukegoto @radeksimko @neildothunter @tombuildsstuff