Skip to content

remove support of hidden_envoy_deprecated_remove_accept_encoding_header#17963

Closed
ankatare wants to merge 1 commit intoenvoyproxy:mainfrom
ankatare:hidden_envoy_deprecated_remove_accept_encoding_header
Closed

remove support of hidden_envoy_deprecated_remove_accept_encoding_header#17963
ankatare wants to merge 1 commit intoenvoyproxy:mainfrom
ankatare:hidden_envoy_deprecated_remove_accept_encoding_header

Conversation

@ankatare
Copy link
Copy Markdown
Contributor

@ankatare ankatare commented Sep 2, 2021

Signed-off-by: Abhay Narayan Katare abhay.katare@india.nec.com

Commit Message: Remove support of hidden_envoy_deprecated_remove_accept_encoding_header
Additional Description: Another PR of type remove_v2_support. first PR is #16274
Risk Level: LOW
Testing: bazel test on test/extensions/filters/http/gzip/..
Docs Changes: NA
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]
[Optional API Considerations:]

Signed-off-by: Abhay Narayan Katare <abhay.katare@india.nec.com>
@ankatare ankatare requested a review from dio as a code owner September 2, 2021 06:29
@ankatare
Copy link
Copy Markdown
Contributor Author

ankatare commented Sep 2, 2021

@phlax not able to hit on button Resolve conflicts (option is disable to me).
cc @tyxia @dio

@tyxia
Copy link
Copy Markdown
Member

tyxia commented Sep 2, 2021

@phlax not able to hit on button Resolve conflicts (option is disable to me).
cc @tyxia @dio

You can do git pull upstream main locally in your repro branch to merge and then resolve the conflict and then git push. I guess you can only hit the button when you have write access? It is disabled for me as well.

But i don't find hidden_envoy_deprecated_remove_accept_encoding_header or even gzip folder in envoyproxy repro. So i think you may can just close this PR after merging main to confirm.

@ankatare
Copy link
Copy Markdown
Contributor Author

ankatare commented Sep 2, 2021

@tyxia Thanks for prompt reply. But ideally i able to resolve conflicts with the "Resolve Conflicts" button. Not sure what happen this time.
Anyway let me try resolving conflicts locally as well.
Have you worked in "hidden_envoy_deprecated_remove_accept_encoding_header" ?

@tyxia
Copy link
Copy Markdown
Member

tyxia commented Sep 2, 2021

@tyxia Thanks for prompt reply. But ideally i able to resolve conflicts with the "Resolve Conflicts" button. Not sure what happen this time.
Anyway let me try resolving conflicts locally as well.
Have you worked in "hidden_envoy_deprecated_remove_accept_encoding_header" ?

I haven't but I cannot find it in code base, appears to be done by someone already.

Yea. I think it is good to keep you local repro update to date. This can helps you avoid duplicated work like this.

@ankatare
Copy link
Copy Markdown
Contributor Author

ankatare commented Sep 2, 2021

@tyxia Thanks for prompt reply. But ideally i able to resolve conflicts with the "Resolve Conflicts" button. Not sure what happen this time.
Anyway let me try resolving conflicts locally as well.
Have you worked in "hidden_envoy_deprecated_remove_accept_encoding_header" ?

I haven't but I cannot find it in code base, appears to be done by someone already.

Yea. I think it is good to keep you local repro update to date. This can helps you avoid duplicated work like this.

@tyxia Humm, it is really bad to me :( . seems second PR today with duplicate work :( . just quick query normally i do "git fetch upstream main" . how it is different from "git pull upstream main" ?

@tyxia
Copy link
Copy Markdown
Member

tyxia commented Sep 2, 2021

git fetch upstream main

git pull does a git fetch followed by a git merge FETCH_HEAD. You need to either git pull or git fetch + git merge to bring your local up-to-date and update your remote as well.

@ankatare
Copy link
Copy Markdown
Contributor Author

ankatare commented Sep 3, 2021

@tyxia you are right and thanks a lot for your help. I am closing this PR.

@ankatare ankatare closed this Sep 3, 2021
@ankatare ankatare deleted the hidden_envoy_deprecated_remove_accept_encoding_header branch September 29, 2021 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants