-
Notifications
You must be signed in to change notification settings - Fork 37
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
Fix - Choose reference over pointer #107
Conversation
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.
Looks good to me, except that I've my opinion for a few. Left to you.
``` | ||
|
||
|
||
**Parameters** | ||
|
||
* DroneCoreImpl * **parent** - | ||
* DroneCoreImpl & **parent** - |
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.
Is there a space between &
and parent ?
I think DroneCoreImpl &parent
looks better than DroneCoreImpl & parent
. What do you think ?
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.
This is what is generated by the toolchain (not this specific change), so IMO we should accept this PR and deal with the rendering as a separate issue.
I'll merge and lets carry on the conversation in mavlink/MAVSDK#321
``` | ||
|
||
**Parameters** | ||
|
||
* [Device](classdronecore_1_1_device.md) * **device** - The specific device associated with this plugin. | ||
* [Device](classdronecore_1_1_device.md) & **device** - The specific device associated with this plugin. |
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.
How about Device &device ?
std::shared_ptr<Action> action = std::make_shared<Action>(&device); | ||
std::shared_ptr<FollowMe> follow_me = std::make_shared<FollowMe>(&device); | ||
std::shared_ptr<Telemetry> telemetry = std::make_shared<Telemetry>(&device); | ||
std::shared_ptr<Action> action = std::make_shared<Action>(device); |
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.
Why not auto action = std::make_shared<Action>(device);
?
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.
No reason at all, except that this is a direct copy of your example code :-)
https://github.com/dronecore/DroneCore/blob/develop/example/follow_me/follow_me.cpp#L53
So if the example changes, so will this.
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.
oh! :) Sorry. I found my mistake!
This fixes up docs to match PR mavlink/MAVSDK#316 (ie addresses #106 )
@shakthi-prashanth-m Can you please sanity check this.