On 07/02/2013 06:49 AM, Amos Jeffries wrote: > This creates an Ssl::Config class to manage SSL configuration settings > and creates a ConfigOptions sub-class to hold the settings which are > needed passing around in the SSL code.
The above description does not match the patch: We already have an Ssl::Config class and the ConfigOptions you are adding is not a subclass of that. > +/// parse and store the configuration options used > +/// for generating an SSL context > +class ConfigOptions Thank you for writing a class description! I suggest renaming this class to Ssl::ContexConfig or Ssl::ContextOptions to distinguish it from the existing Ssl::Config class. However, if this class is actually not specific to SSL context (I have not checked) but is specific to a "port" (as in http_port or https_port), then something like Ssl::PortOptions would be better. We do not have an outgoing "port" configuration option, but we kind of have that concept so it may work for both outgoing and incoming connections. I would also remove "parse and store the" from the description to avoid limiting it too much and to focus on the class meaning rather than a few of its actions. For example: /// manages SSL context configuration options class ContextConfig > TODO: > * implement dump/free functions properly > * implement parse for backward-compatible loading of the old DIRECT SSL > settings options. * implement assignment operator for the new class Thank you, Alex.
