Code review comment for lp:~vjsamuel/drizzle/rplugin-drizzle-protocol

Revision history for this message
Monty Taylor (mordred) wrote :

Good work so far. Now that you're hacking in the server, there are a few more new things you're going to need to deal with. (Most of this has to do with error message handling)

First of all: in init(module::Context &context), you're doing validation checks directly at module init time. I'd really prefer to see option value validation happen at option parse time, so that we don't have to get halfway in to server startup before we discover a bad value in a config file. Can you register validation methods for these values when you add them to the context?

Secondly, for reporting error messages, you need to use errmsg_printf rather than cout. This allows them to be processed by the servers error message reporting system, which may or may not be configured to actually print to a console (other options would be syslog or the like)

Just while I'm giving you feedback, if you _were_ to use a cout approach (not a choice here) you should be using cerr instead, and also should be adding a std::endl on to the end.

Finally, you need to wrap these new messages strings you just added in _() so that they'll get translated.

So this:

cout << "Invalid value for buffer_length";

Should become:

errmsg_printf(ERRMSG_LVL_ERROR, _("Invalid value for buffer_length\n"));

review: Needs Fixing

« Back to merge proposal