I made one more pass through the unittests library and it looks fine, but I still find it wanting. Overall:

* From too few examples now it has too many! The fine line is somewhere in between (I'd say on the short side). I complained about the initial documentation because the example were meaningless (e.g. they used names that weren't defined. The current iteration of the library gives so many examples it's patronizing. Yes, if you give the list of operators supported, you don't need to give an example of each! We're not oligophrenes over here.

* To add insult to injury, examples have a double-spacing that is a bit much.

* The code has a lot of lines beyond 80 characters. 80 characters should be plenty to write good code, and makes for readable code that doesn't need to occupy one's entire screen. I wish I could somehow convince everybody to not complain, not debate, and not show me the old style guide. Let's just write 80-columns code. That includes documentation. Please. Let's.

* Even examples are sometimes too wide - some don't fit in my browser unless I enlarge it considerably.

* I don't mean to belittle the effort, but the documentation needs one more pass for typos etc. I wouldn't mention this if I hadn't noticed Jonathan is otherwise a perfect speller and a good writer. So, it's "Generally speaking" not "Generally-speaking", "given set" not "give set"... and that's just the first line. Eliminate all flowery: there's no need for "generally speaking" and "in essence" "particularly" etc. etc. etc.
        
* For inline code use $(D ) not anything else

* I appreciate the date-based examples but I'd rather stick with one focus at a time.

* There's got to be too much of a good thing. The massive unittests, the lavish waste of vertical space (import with commas, anyone?) and the documentation make adding four simple concepts a 1887 lines deal. I guess that's borderline okay but I am increasingly hawkish about massive additions of small functionality to Phobos.

* The documentation for the mock-up assertPred could be moved up to the module documentation, which has the advantage that it avoids any self-explaining: "There is further documentation for each version below (this particular version of the function doesn't actually exist - it's here so that there's a good spot to put documentation for the function as a whole)." Why would anyone need to absorb that kind of information in order to use four simple concepts?

My vote is in favor to adopting this library, contingent upon (a) making a spelling and correctness pass and (b) giving the documentation a thorough haircut. The rest of my comments are optional as they could be effected later.



Andrei

Reply via email to