Code review comment for lp:~gandelman-a/charm-helpers/filter_req_packages

Revision history for this message
Matthew Wedgwood (mew) wrote :

I think this is a good addition, but I have some (perhaps nitpicky) problems with it as-is:

1. The function name is a little opaque. "Required" is a little vague. Also, you're actually filtering the packages that aren't required. Perhaps filter_installed_packages would be ok?

2. The function assumes (by throwing an exception) that the package cache is up-to-date. It makes sense to me simply to filter the packages that are already installed, returning everything else. That way you could run 'apt-get update' later if an un-installed packaged required a new repo. This should still fit your use case, as an exception would be thrown by apt_install when the package wasn't found.

review: Needs Fixing

« Back to merge proposal