pkgme

Merge lp://staging/~james-w/pkgme/python-dependencies into lp://staging/pkgme

  1. python-dependencies
  2. Merge into trunk
Proposed by James Westby
Status: Merged
Merged at revision: 61
Proposed branch: lp://staging/~james-w/pkgme/python-dependencies
Merge into: lp://staging/pkgme
Prerequisite: lp://staging/~james-w/pkgme/clean-not-validate
Diff against target: 236 lines (+97/-11)
8 files modified
pkgme/distutils_command/pkgme_info.py (+20/-2)
pkgme/info_elements.py (+18/-0)
pkgme/package_files.py (+2/-0)
pkgme/templates/rules (+3/-0)
pkgme/tests/test_distutils_command.py (+21/-0)
pkgme/tests/test_info_elements.py (+20/-1)
pkgme/tests/test_package_files.py (+10/-7)
setup.py (+3/-1)
To merge this branch: bzr merge lp://staging/~james-w/pkgme/python-dependencies
Reviewer Review Type Date Requested Status
Jonathan Lange Approve
pkgme committers Pending
Review via email: mp+67325@code.staging.launchpad.net

Description of the change

Hi,

This branch adds a couple of new features to the Python backend in order to
have Build-Depends and Depends filled in correctly.

In testing this I found that pkgme's stated dependencies were incomplete, so
it fixes that too.

Thanks,

James

To post a comment you must log in.
Revision history for this message
Jonathan Lange (jml) wrote :

I think I follow this.

Not sure why build_depends concatenates a string rather than appending to a list and returning ', '.join(dependencies), but it's not a big deal.

Is there a way to avoid hard-coding "/usr/share/python"? I would have thought it was unnecessary after adding the extra dependencies to setup.py.

More generally, I find the 'getattr(self, method_name)()' pattern easier to follow if there's a standard prefix for those methods. (e.g. 'test_', 'cmd_', etc.) It would be churlish to ask you to change it in this branch.

review: Needs Information
Revision history for this message
James Westby (james-w) wrote :

> I think I follow this.
>
> Not sure why build_depends concatenates a string rather than appending to a
> list and returning ', '.join(dependencies), but it's not a big deal.

It's mainly that it gets a string as the input. It would still work your way
though, so I'll change it.

> Is there a way to avoid hard-coding "/usr/share/python"? I would have thought
> it was unnecessary after adding the extra dependencies to setup.py.

Actually you raise an interesting point. This isn't provided by python-debian,
it's actually part of the python packages on Debian/Ubuntu (and nowhere else I think).
That's obviously not very portable, but it was the closest thing to hand to get
this functionality.

A better solution is likely to be to re-implement that logic so that it is
portable.

Also, is /usr/share/python usually on sys.path?

> More generally, I find the 'getattr(self, method_name)()' pattern easier to
> follow if there's a standard prefix for those methods. (e.g. 'test_', 'cmd_',
> etc.) It would be churlish to ask you to change it in this branch.

It would be pretty easy to do though.

Thanks,

James

Revision history for this message
Jonathan Lange (jml) wrote :

On Fri, Jul 8, 2011 at 9:05 PM, James Westby <email address hidden> wrote:
>> I think I follow this.
>>
>> Not sure why build_depends concatenates a string rather than appending to a
>> list and returning ', '.join(dependencies), but it's not a big deal.
>
> It's mainly that it gets a string as the input. It would still work your way
> though, so I'll change it.
>

Ta.

>> Is there a way to avoid hard-coding "/usr/share/python"? I would have thought
>> it was unnecessary after adding the extra dependencies to setup.py.
>
> Actually you raise an interesting point. This isn't provided by python-debian,
> it's actually part of the python packages on Debian/Ubuntu (and nowhere else I think).
> That's obviously not very portable, but it was the closest thing to hand to get
> this functionality.
>
> A better solution is likely to be to re-implement that logic so that it is
> portable.
>

OK, that makes sense. It's probably worth adding a comment along these lines.

> Also, is /usr/share/python usually on sys.path?
>

No. Or at least, not as far as I can tell.

>> More generally, I find the 'getattr(self, method_name)()' pattern easier to
>> follow if there's a standard prefix for those methods. (e.g. 'test_', 'cmd_',
>> etc.) It would be churlish to ask you to change it in this branch.
>
> It would be pretty easy to do though.

Yeah :)

I'd be happy to do a follow-up branch for this.

jml

Revision history for this message
Jonathan Lange (jml) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: