Thread overview
[phobos] Parallel reviews of TempAlloc and std.path?
May 31, 2011
Robert Jacques
May 30, 2011
My proposal for a new std.path is now ready for review, and it is my understanding that the same is true for David's TempAlloc.  Would it be OK if we run the reviews in parallel?  The two are completely independent, and AFAIK TempAlloc is meant for druntime, not Phobos.


For the curious/impatient:

https://github.com/kyllingstad/phobos/blob/new-std-path/std/path.d http://www.kyllingen.net/code/new-std-path/phobos-prerelease/std_path.html

-Lars

May 30, 2011
On Mon, May 30, 2011 at 10:38 AM, Lars Tandle Kyllingstad <lars at kyllingen.net> wrote:
> My proposal for a new std.path is now ready for review, and it is my understanding that the same is true for David's TempAlloc. ?Would it be OK if we run the reviews in parallel? ?The two are completely independent, and AFAIK TempAlloc is meant for druntime, not Phobos.
>
>
> For the curious/impatient:
>
> https://github.com/kyllingstad/phobos/blob/new-std-path/std/path.d http://www.kyllingen.net/code/new-std-path/phobos-prerelease/std_path.html
>
> -Lars
>

Quickly looked at the doc. Why is this true in glob?

---
assert (glob("foobar", "foo?bar"));
---

The doc says "?	Matches exactly one instances of any character"

While you have the patient open can you fix the existing documentation? What is "baseName("dir/file.ext")            --> "file.ext"? We should use assert. '-->' is not a D construct.
May 30, 2011
On Mon, 2011-05-30 at 17:44 -0300, Jose Armando Garcia wrote:
> On Mon, May 30, 2011 at 10:38 AM, Lars Tandle Kyllingstad <lars at kyllingen.net> wrote:
> > My proposal for a new std.path is now ready for review [...]

I appreciate your input, but I didn't really mean for the review to start here and now.  Rather, it was meant as a request to start the review in the main D newsgroup.  Therefore, I'll only give very brief answers to your questions, and we can postpone the full discussion until the review starts.


> Quickly looked at the doc. Why is this true in glob?
> 
> ---
> assert (glob("foobar", "foo?bar"));
> ---
> 
> The doc says "?	Matches exactly one instances of any character"

Seems like a bug.  I didn't write glob(), that is from the old std.path,
where it was called fnmatch().  I'll look into it.


> While you have the patient open can you fix the existing documentation? What is "baseName("dir/file.ext")            --> "file.ext"? We should use assert. '-->' is not a D construct.

That is (more or less) how the examples in the current std.path are written.  Maybe it should be changed, now that it seems we are getting "compile & run" buttons in the web documentation.

-Lars

May 31, 2011
On Mon, 2011-05-30 at 17:44 -0300, Jose Armando Garcia wrote:
> On Mon, May 30, 2011 at 10:38 AM, Lars Tandle Kyllingstad <lars at kyllingen.net> wrote:
> > My proposal for a new std.path is now ready for review [...]
> 
> Quickly looked at the doc. Why is this true in glob?
> 
> ---
> assert (glob("foobar", "foo?bar"));
> ---
> 
> The doc says "?	Matches exactly one instances of any character"

Fortunately, the example was wrong.  The code works as expected.  I have made some improvements to the glob() documentation now.  Thanks for pointing this out!

-Lars

May 31, 2011
On Mon, 30 May 2011 09:38:31 -0400, Lars Tandle Kyllingstad <lars at kyllingen.net> wrote:
> My proposal for a new std.path is now ready for review, and it is my understanding that the same is true for David's TempAlloc.  Would it be OK if we run the reviews in parallel?  The two are completely independent, and AFAIK TempAlloc is meant for druntime, not Phobos.
>
>
> For the curious/impatient:
>
> https://github.com/kyllingstad/phobos/blob/new-std-path/std/path.d http://www.kyllingen.net/code/new-std-path/phobos-prerelease/std_path.html
>
> -Lars

The reason reviews are run serially has nothing to do with whether they are independent or not; it has everything to do with the limited number of eyeballs available.