http: prefer to return reference rather than shared pointer for virtualhost()#44048
Conversation
…alHost() Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>
ravenblackx
left a comment
ravenblackx
left a comment
There was a problem hiding this comment.
Probably would be worth having a relnote about all of these changes, in one of these PRs (or on its own afterwards), so that anyone who has custom extensions has a heads-up about needing to replace -> with . a lot (or to give the function +SharedPtr for a rare case).
| return vhost_copy_; | ||
| } | ||
| const VirtualHost& virtualHost() const override { return *vhost_; } | ||
| VirtualHostConstSharedPtr virtualHostSharedPtr() const override { return vhost_copy_; } |
There was a problem hiding this comment.
| VirtualHostConstSharedPtr virtualHostSharedPtr() const override { return vhost_copy_; } | |
| VirtualHostConstSharedPtr virtualHostSharedPtr() const override { return vhost_; } |
And delete vhost_copy_ from the class and the constructor, it's no longer necessary because of not returning a reference to a shared_ptr.
Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>
ｉguess the modern IDE should will give a clear suggestion once the developers encounter a compilation problem. But i won't against the note if you prefer it. Where will you suggest to add the note? Change log? Or a message at envoy-dev channel? |
|
/retest |
I know I, as a person who maintains a bunch of custom filters, would hear about it from the compilation problem more than from the changelog, but I might look at the changelog to see why it changed - and a note there would be good to make me think about whether I want SharedPtr or OptRef option, vs. if I just got the compiler complaint I would probably just pick one and do it globally without really thinking about which option makes sense. (Probably in the |
…alhost() (envoyproxy#44048) Commit Message: http: prefer to return reference rather than shared pointer for virtualhost() Additional Description: See envoyproxy#44025 for our new style. Risk Level: n/a. Testing: n/a. Docs Changes: n/a. Release Notes: n/a. Platform Specific Features: n/a. --------- Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>
…alhost() (envoyproxy#44048) Commit Message: http: prefer to return reference rather than shared pointer for virtualhost() Additional Description: See envoyproxy#44025 for our new style. Risk Level: n/a. Testing: n/a. Docs Changes: n/a. Release Notes: n/a. Platform Specific Features: n/a. --------- Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com> Signed-off-by: Xuyang Tao <taoxuy@google.com>
…alhost() (envoyproxy#44048) Commit Message: http: prefer to return reference rather than shared pointer for virtualhost() Additional Description: See envoyproxy#44025 for our new style. Risk Level: n/a. Testing: n/a. Docs Changes: n/a. Release Notes: n/a. Platform Specific Features: n/a. --------- Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com> Signed-off-by: Jonathan Wu <jtwu@google.com>
…alhost() (envoyproxy#44048) Commit Message: http: prefer to return reference rather than shared pointer for virtualhost() Additional Description: See envoyproxy#44025 for our new style. Risk Level: n/a. Testing: n/a. Docs Changes: n/a. Release Notes: n/a. Platform Specific Features: n/a. --------- Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com> Signed-off-by: Nick Shipilov <nick.shipilov.n@gmail.com>
…alhost() (envoyproxy#44048) Commit Message: http: prefer to return reference rather than shared pointer for virtualhost() Additional Description: See envoyproxy#44025 for our new style. Risk Level: n/a. Testing: n/a. Docs Changes: n/a. Release Notes: n/a. Platform Specific Features: n/a. --------- Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>
5 participants
Commit Message: http: prefer to return reference rather than shared pointer for virtualhost()
Additional Description:
See #44025 for our new style.
Risk Level: n/a.
Testing: n/a.
Docs Changes: n/a.
Release Notes: n/a.
Platform Specific Features: n/a.