[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: RFS: shedskin



Jakub Wilk wrote:
>
> I don't intend to sponsor this package, but here's my review:

Thanks for your time!

> The patches look like if "Index: ..." was a part of patch header. I think
> this is not what you meant.[0]

Right. I'll make some cleaner patches using revised quilt options.

> You can't use regular expressions in Files fields of a DEP-5 copyright
> file.

Really? That isn't what I understand from...

http://dep.debian.net/deps/dep5/#files-files-field

I suppose that the following is asking a bit much...

Files: shedskin/lib/(itertools|heapq).?pp

...but surely the following should be OK:

Files: shedskin/lib/itertools.?pp shedskin/lib/heapq.?pp

> I'd replace "Copyright: Copyright" with simply "Copyright:".

Good point: I don't know what I was thinking there. :-)

> I don't think that "Any feedback is very welcome" is an appropriate thing
> to put into copyright file.

This was a comment from the original copyright file associated with the work, 
but I imagine that it could be moved somewhere else or removed.

> You probably want to add a paragraph about files in debian/ directory to
> the copyright file.

OK, but I did have a comment like this...

Comment: This package was debianized by Paul Boddie <paul@boddie.org.uk>.

I guess you're referring to a specific copyright section for those files.

> Is there any reason you list 8 different wildcards in debian/*.install,
> when what you really do is to install all files from a directory? You could
> have put just the directory name there...

It might be historical, but I don't see any good reason for not just using the 
directory name if that will bring in all the files and directories 
recursively, so I'll change this.

> Some aesthetical nitpicking: in debian/control, I'd put a space between
> ">=" and "7.0.50~", and after "Homepage:".

Right.

> I'd nice if you could move FLAGS* to a directory that's not in sys.path.

I could look into a patch that moves it elsewhere, but I imagine that it's 
placed within a package as package data for convenient access. Any 
suggestions for a different location?

> Are the Python modules that are currently public supposed to be used by
> other software? If not, I guess it'd make sense to move them into a private
> directory, too.

I'm not sure. I can imagine some re-use, but I don't know of anybody using the 
skedskin package in that way.

> Will the .py files in /usr/share/shedskin/lib be ever used by a "normal"
> Python interpreter?

I don't think so: they are the standard/runtime library files for shedskin.

> shedskin/inter.py has:
>
> import gc, random
> random.seed(42)
>
> Is that intentional?

I'll have to ask the upstream author about that.

Thank you again for taking the time to look at the package. I'll try and make 
the changes suggested, re-package and re-post, and hopefully get closer to 
finding a sponsor.

Paul


Reply to: