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

[Breaking] Remove ACL rules with regex #4360

Merged
merged 1 commit into from
Dec 9, 2019
Merged

[Breaking] Remove ACL rules with regex #4360

merged 1 commit into from
Dec 9, 2019

Conversation

animesh2049
Copy link
Contributor

@animesh2049 animesh2049 commented Dec 5, 2019 •
edited
Loading

Regex rules were confusing and our power customers were not using it, hence removing it now.

This change is Reviewable

@animesh2049 animesh2049 requested review from manishrjain and a team as code owners December 5, 2019 13:15
manishrjain
manishrjain approved these changes Dec 5, 2019
View reviewed changes
Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 6 of 6 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @mangalaman93 and @martinmr)

manishrjain
manishrjain approved these changes Dec 5, 2019
View reviewed changes
Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

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

Do add a [Breaking] prefix to the title. Replace "Depricate" with "Remove".

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @mangalaman93 and @martinmr)

mangalaman93
mangalaman93 requested changes Dec 6, 2019
View reviewed changes
Copy link
Contributor

@mangalaman93 mangalaman93 left a comment

Choose a reason for hiding this comment

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

We should update the corresponding documentation before we merge this PR.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @mangalaman93 and @martinmr)

@mangalaman93 mangalaman93 self-requested a review December 6, 2019 07:06
mangalaman93
mangalaman93 reviewed Dec 6, 2019
View reviewed changes
Copy link
Contributor

@mangalaman93 mangalaman93 left a comment

Choose a reason for hiding this comment

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

I am also wondering why we decided to remove regexp, a reference reasoning in the commit message would be nice.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @mangalaman93 and @martinmr)

@mangalaman93 mangalaman93 self-requested a review December 6, 2019 07:07
@animesh2049 animesh2049 changed the title Depricate ACL rules with regex [Breaking] Remove ACL rules with regex Dec 6, 2019
@animesh2049
Copy link
Contributor Author

I am also wondering why we decided to remove regexp, a reference reasoning in the commit message would be nice.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @mangalaman93 and @martinmr)

Please check the updated PR description.

@animesh2049 animesh2049 merged commit d41e863 into master Dec 9, 2019
@animesh2049 animesh2049 deleted the animesh2049/remove_acl_regex branch December 9, 2019 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@manishrjain manishrjain manishrjain approved these changes

@martinmr martinmr Awaiting requested review from martinmr

@mangalaman93 mangalaman93 Awaiting requested review from mangalaman93

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@animesh2049 @manishrjain @mangalaman93