2008/6/2 Danek Duvall <[EMAIL PROTECTED]>:
> On Sun, Jun 01, 2008 at 10:27:13PM -0500, Shawn Walker wrote:
>
>> http://cr.opensolaris.org/~swalker/pkg-depot-9/
>
> catalog.py:
>
>  - line 1167: why the return statement here?

Perl habits die hard :-)

Changed.

> repository.py:
>
>  - line 90: you'll end up with two adjacent spaces here.

Fixed.

>  - line 101: capitalize first word of sentence.

Fixed.

>> >  - line 141: I'm confused on how we might get here.  It seems like no
>> >    matter what you do, unless you get shunted into the face code above,
>> >    we're going to error out of this method.
>>
>> I think you're right; it looks like a leftover from the old logic.
>
> I'm still confused here.  If we don't go into the face code, does this
> method do anything useful other than return errors?  What happens if line

No. It is strictly for error handling.

> 142 doesn't fire?  Is there any point in the code in this method from line

Because of line 121, 142 should always fire if we get there.

As a result, I have removed the op_method and "if" condition. The rest
should still do the right thing.

> 131 onwards?

Yes, there is. If you request:

http://localhost:10000/search/1/

...it will return a message telling you that version 1 is not
supported for search.

Whereas if you request:

http://localhost:10000/foo/1/

You'll get face.unknown().

> pkginfo:
>
>  - line 28: Huh?  I suppose you could simply say "all" here, but we
>    typically don't tag packages as being architecture neutral, even if
>    they are.

Sorry, I wasn't aware of that. CherryPy is architecture neutral.

I had gotten the "ANY" value from a pkginfo I found on src.opensolaris.org.

However, I have changed this to "ISA" as it was previously.

>> > SUNWpython-cherrypy/Makefile:
>> >
>> >  - If nothing else, your "prototype" target rule should be broken up into
>> >    more rules.
>>
>> I'm not sure what you wanted, but I have split it up and simplified it
>> some more.
>
> That's better.  I'd probably go one step further and put the cherrypy build
> in its own target as well.  Right now, it's going to rebuild cherrypy every
> single time you type make install.  I'm not too annoyed by this yet, though,
> so maybe I'll be nice and fix it myself sometime later.

I did that on purpose (due to how often I was doing make clobber and
trying to get things right during development), but I'm not opposed to
changing it.

I've got it split up now so that it won't build the package for
CherryPy or install CherryPy unless the expected files are missing.

I also fixed a few warnings I was getting from pkgmk because I missed
three or four directories in the prototype (oops).

I hope the new version is better?

Updated webrev:
http://cr.opensolaris.org/~swalker/pkg-depot-10/

Diff since last webrev:
http://cr.opensolaris.org/~swalker/pkg-depot-10/raw_files/pkg-depot-10.patch

Thanks for your patience, I would prefer to get this as close to right
the first time as possible.

Cheers,
-- 
Shawn Walker
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to