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

remove well_known_names.h files #7238

Open
mattklein123 opened this issue Jun 11, 2019 · 12 comments · Fixed by #16164
Open

remove well_known_names.h files #7238

mattklein123 opened this issue Jun 11, 2019 · 12 comments · Fixed by #16164
Assignees
@theshubhamp
Labels
beginner Good starter issues! help wanted Needs help! tech debt

Comments

@mattklein123
Copy link
Member

These files are present for historical reasons and do not serve any real purpose. All extension code should be completely contained within the extension and that includes the name of the extension. We should remove all of these files.
@mattklein123 mattklein123 added tech debt help wanted Needs help! beginner Good starter issues! labels Jun 11, 2019
@moderation
Copy link
Contributor

Do the files below remain with everything under source/extensions/*/well_known_name.h removed?

source/common/config/well_known_names.cc
source/common/config/well_known_names.h

@mattklein123
Copy link
Member Author

@moderation sorry yeah I think those should stay.

@theshubhamp
Copy link
Contributor

Hello! I'd like to work on this if no one has picked this up yet.

@theshubhamp theshubhamp mentioned this issue Oct 22, 2019
Closed
@kushthedude
Copy link

I would like to work on it

@kushthedude kushthedude mentioned this issue Nov 11, 2020
Closed
@asraa asraa mentioned this issue Jan 20, 2021
Closed
@alyssawilk alyssawilk mentioned this issue Feb 24, 2021
Merged
@daixiang0 daixiang0 mentioned this issue Apr 26, 2021
Merged
@daixiang0
Copy link
Member

Now find those files, which one should stay:

root:[envoy]$ find -name well_known_names.h
./source/common/config/well_known_names.h
./source/extensions/internal_redirect/well_known_names.h
./source/extensions/common/wasm/well_known_names.h
./source/extensions/transport_sockets/well_known_names.h
./source/extensions/transport_sockets/tls/cert_validator/well_known_names.h
./source/extensions/resource_monitors/well_known_names.h
./source/extensions/access_loggers/well_known_names.h
./source/extensions/filters/network/rocketmq_proxy/well_known_names.h
./source/extensions/filters/network/well_known_names.h
./source/extensions/filters/network/thrift_proxy/filters/well_known_names.h
./source/extensions/filters/network/dubbo_proxy/filters/well_known_names.h
./source/extensions/filters/listener/well_known_names.h
./source/extensions/filters/http/well_known_names.h
./source/extensions/grpc_credentials/well_known_names.h
./source/extensions/stat_sinks/well_known_names.h
./source/extensions/health_checkers/well_known_names.h
./source/extensions/retry/priority/well_known_names.h
./source/extensions/clusters/well_known_names.h

@daixiang0
Copy link
Member

@mattklein123 @yanavlasov some known_names files still exist, need to fix?

@mattklein123 mattklein123 reopened this May 27, 2021
@daixiang0
Copy link
Member

/assign @daixiang0

This was referenced May 27, 2021
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Closed
This was referenced Jun 3, 2021
Merged
Merged
Merged
zuercher pushed a commit that referenced this issue Jun 8, 2021
@daixiang0
thrift_proxy: remove well_known_names (#16837)
4ae47c5 
Remove well_known_names in thrift_proxy field, part of #7238

Risk Level: Low
Testing: functional
Docs Changes: n/a
Release Notes: n/a
Signed-off-by: Long Dai <[email protected]>
zuercher pushed a commit that referenced this issue Jun 10, 2021
@daixiang0
filters/http: remove well_known_names (#16871)
9dd8ae3 
Remove well_known_names in filters/http field, part of #7238

Risk Level: Low
Testing: functional
Release Notes: n/a
Signed-off-by: Long Dai <[email protected]>
@daixiang0 daixiang0 mentioned this issue Jun 16, 2021
Closed
@arvryna
Copy link

arvryna commented Sep 12, 2021

hi, is this done ? or some more files needed to be removed ?

@daixiang0
Copy link
Member

still need.

@arvryna
Copy link

arvryna commented Sep 13, 2021

@daixiang0 , you want to continue working on this ?

@daixiang0 daixiang0 mentioned this issue Sep 17, 2021
Merged
leyao-daily pushed a commit to leyao-daily/envoy that referenced this issue Sep 30, 2021
@daixiang0
thrift_proxy: remove well_known_names (envoyproxy#16837)
a102768 
Remove well_known_names in thrift_proxy field, part of envoyproxy#7238

Risk Level: Low
Testing: functional
Docs Changes: n/a
Release Notes: n/a
Signed-off-by: Long Dai <[email protected]>
leyao-daily pushed a commit to leyao-daily/envoy that referenced this issue Sep 30, 2021
@daixiang0
filters/http: remove well_known_names (envoyproxy#16871)
67083b2 
Remove well_known_names in filters/http field, part of envoyproxy#7238

Risk Level: Low
Testing: functional
Release Notes: n/a
Signed-off-by: Long Dai <[email protected]>
@daixiang0
Copy link
Member

@arvryna no, you can go ahead.

@daixiang0 daixiang0 removed their assignment Feb 20, 2023
@siddharthkhonde
Copy link

@mattklein123 is this still open? I can take a look into this as a beginner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@theshubhamp theshubhamp

Labels
beginner Good starter issues! help wanted Needs help! tech debt
Projects
None yet
Milestone
No milestone
7 participants
@moderation @arvryna @mattklein123 @daixiang0 @kushthedude @theshubhamp @siddharthkhonde