-
Notifications
You must be signed in to change notification settings - Fork 648
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
silencing the unused lambda capture warning #853
silencing the unused lambda capture warning #853
Conversation
libraries/app/api.cpp
Outdated
@@ -151,7 +151,7 @@ namespace graphene { namespace app { | |||
{ | |||
auto block_num = b.block_num(); | |||
auto& callback = _callbacks.find(id)->second; | |||
fc::async( [capture_this,this,id,block_num,trx_num,trx,callback]() { | |||
fc::async( [capture_this,id,block_num,trx_num,trx,callback]() { |
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.
I have been working on this line of code for another PR (#813), and can verify that the change above causes no side effects. Unfortunately it doesn't fix 813's problem, but that is another story...
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 for confirming that @jmjatlanta
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.
I want to hear more discussion about this. @pmconrad ?
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.
Removing "this" shouldn't make a difference. Nevertheless I think we should wait for John's results from #813 before changing 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.
looks good to me.
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.
@ihla please revert the change about api.cpp then we'll merge other changes in this PR.
@jmjatlanta please consider removing this
in #813 if it's OK to do so.
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.
No description provided.