-
Notifications
You must be signed in to change notification settings - Fork 3k
Ensure all overridden virtual functions are marked with override
#13042
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
Conversation
@hugueskamba, thank you for your changes. |
features/lwipstack/LWIPStack.h
Outdated
@@ -341,7 +341,7 @@ class LWIP : public OnboardNetworkStack, private mbed::NonCopyable<LWIP> { | |||
nsapi_error_t add_dns_server(const SocketAddress &address, const char *interface_name) override; | |||
|
|||
/** @copydoc NetworkStack::get_ip_address */ | |||
virtual nsapi_error_t get_ip_address(SocketAddress *address); | |||
virtual nsapi_error_t get_ip_address(SocketAddress *address) override; |
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.
Style preference is to drop the virtual
- only use one of virtual
, override
, final
. (Even though each implies the previous). https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rh-override
That's the way the rest of the file is written.
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.
Thanks. Done.
This also removes warnings.
940063b
to
1beb8d3
Compare
CI started |
Test run: SUCCESSSummary: 7 of 7 test jobs passed |
This PR does not contain release version label after merging. |
Summary of changes
This also removes warnings.
Impact of changes
N/A
Migration actions required
N/A
Documentation
N/A
Pull request type
Test results
Reviewers
@kjbracey-arm