Review of std.net.isemail part 2

[Andrei Alexandrescu]

On 3/29/11 4:06 PM, Jacob Carlborg wrote:
> I've made a few minor changes:
>
> * Renamed EmailStatusCode.Off -> None and On -> Any
> * Added and clarified the documentation for EmailStatusCode.Any and None
> * Updated the documentation
>
> Github: https://github.com/jacob-carlborg/phobos/tree/isemail
> Docs: http://dl.dropbox.com/u/18386187/isemail.html

Overall I think the module is in good shape for admission, with the nits 
below. Here is my review:

* First doc sentence: "To validate..." is grammatically mistaken 
(doesn't have a predicate). Normally I'd just note that and fix later, 
but this is the first sentence so I guess it gets a higher priority.

* Copyright: please look up and use the comment convention used 
elsewhere in Phobos.

* Instead of using a bool for checkDns I suggest enum CheckDns { no, yes 
} which is used in other parts of Phobos.

* Line 67: Please use template constraints instead of static assert.

* Line 79 and others: I wish there were no empty line before the "else" 
clause...

* Line 86: There is a constant Threshold that doesn't follow common 
conventions.

* Line 189: enforce?

* Line 235: At best 33, 126, and 10 were symbolic constants or otherwise 
explained.

* Line 920: EmailStatus should be either private or documented.

* Line 1316: Why is Token a struct with an enum in it instead of an 
enum? (Also the static: label is unnecessary.)

* Line 1357: Why is there a need for conversion to int? Wouldn't dchar 
work well?


Andrei