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

Terminate block production loop when shutting down witness plugin #1314 #1364

Merged
merged 3 commits into from Oct 12, 2018

Conversation

Projects
None yet
4 participants
@cogutvalera
Member

cogutvalera commented Oct 10, 2018

Terminate block production loop when shutting down witness plugin #1314

handle enumeration value in switch (block_production_loop)

@abitmore

This comment has been minimized.

Member

abitmore commented Oct 10, 2018

I don't think this is necessary. If want the ilog, we can just add it in the first place.

@abitmore

This comment has been minimized.

Member

abitmore commented Oct 10, 2018

Oh, this is to fix the switch warning. Then why there is no default?

@cogutvalera

This comment has been minimized.

Member

cogutvalera commented Oct 10, 2018

Thanks ! Going to add default now.

ilog( "shutdown producing block" );
return result;
default:
elog( "unknown condition while producing block" );

This comment has been minimized.

@pmconrad

pmconrad Oct 11, 2018

Please add the actual result to the log message.

This comment has been minimized.

@cogutvalera

cogutvalera Oct 11, 2018

Member

ok sure ! Thanks !
Do you mean default case only ? Or both cases with shutdown also ?

Thanks !

This comment has been minimized.

@pmconrad

pmconrad Oct 11, 2018

Only the default case. In the other cases the message is sufficient to identify the case.

This comment has been minimized.

@cogutvalera

cogutvalera Oct 11, 2018

Member

ok. Thanks !

@pmconrad

This comment has been minimized.

pmconrad commented Oct 11, 2018

The default clause has the downside that it will prevent a similar future issue from being detected at compile-time. It is therefore not always a good idea to add one.
In this specific case, logging an error is ok.

@cogutvalera

This comment has been minimized.

Member

cogutvalera commented Oct 11, 2018

@pmconrad I'm absolutely agree with you that default case won't prevent a similar issue at compile-time except this specific case with error log, absolutely agree, that's why I've added error log.

@ryanRfox ryanRfox added this to the 201810 - Feature Release milestone Oct 12, 2018

@pmconrad

Thanks!

@cogutvalera

This comment has been minimized.

Member

cogutvalera commented Oct 12, 2018

Thanks !

@pmconrad pmconrad merged commit 88167ad into bitshares:develop Oct 12, 2018

2 checks passed

ci/dockercloud Your tests passed in Docker Cloud
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment