-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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.
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
stats: change all remaining Stats::ScopePtr references to Stats::ScopeSharedPtr #20888
Conversation
Signed-off-by: Joshua Marantz <[email protected]>
Signed-off-by: Joshua Marantz <[email protected]>
/retest |
Retrying Azure Pipelines: |
/retest |
Retrying Azure Pipelines: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
For the switch what are you waiting for exactly? Perhaps marking the old typedef as deprecated could be a good approach to warning other repos. |
`deprecated` sgtm -- am also going to add a lint check.
There are refs in Nighthawk and a few in Google's private repo which I will
fix; there may be others in some codebases I can't see.
…On Tue, Apr 19, 2022 at 9:10 PM Kevin Baichoo ***@***.***> wrote:
For the switch what are you waiting for exactly? Perhaps marking the old
typedef as deprecated
<https://en.cppreference.com/w/cpp/language/attributes/deprecated> could
be a good approach to warning other repos.
—
Reply to this email directly, view it on GitHub
<#20888 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAO2IPMRH7D2UUSD5J7NBSDVF5KRXANCNFSM5TZUSB4Q>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
|
#20896 adds the lint check and deprecated warning, though that warning doesn't seem to come out for me when I run bazel build on a file where I've used the old name. |
alas, we silence deprecated warning on linux: #19468 |
Yeah I think it's still good as documentation though. In theory clang-tidy should catch it, but it doesn't. |
…eSharedPtr (envoyproxy#20888) Commit Message: Follow-up to envoyproxy#19791, envoyproxy#19790 and envoyproxy#20871, finishing off all remaining ScopePtr references. One more PR to go which will be to remove the ScopePtr definition, but first need to make sure this doesn't break any references outside the repo. Testing: //test/... Docs Changes: n/a Release Notes: n/a Platform Specific Features: n/a Signed-off-by: Joshua Marantz <[email protected]>
Commit Message: Follow-up to #19791, #19790 and #20871, finishing off all remaining ScopePtr references. One more PR to go which will be to remove the ScopePtr definition, but first need to make sure this doesn't break any references outside the repo.
Testing: //test/...
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a