Hi Tim,
General comment:
It might be worth breaking out the code to insert the default search path into one function and using it in all the modules. I think it's used in elf.py, depthlimited.py (and the 24 version though it can't be imported there probably). Since it's only in two places, leaving it as a copy is also fine with me.

pkgdepend.1.txt:
I think some examples of using pkg.depend.bypass-generate would be helpful for users.

flavor/elf.py:
160: should this line still refer to kernel modules or is it used for all elf files?

publish/dependencies.py:
166: If I understand what ignore_bypass is actually doing, I think the docstring might be a bit clearer as: "... determines whether to respect tags which state that generation of dependences against certain files or directories should be bypassed. ..."

319-320:
This is pretty much personal preference, but I would like it better if __verify_run_path returned the list of errors, and that was added to the existing elist if there were any errors on the return on line 320. If your preference runs differently, no worries, just a suggestion.

426-431:
I've reread this a few times, and I still can't wrap my head around it. Maybe an example illustrating the complex case would make things clearer?

470-472:
I can't quite figure out what this comment's trying to explain.

519: How could the basename not be in dep.base_names? (other than it being '*', but I think 448 ensures it can't be '*').

522: Same as 519 for dir and dep.run_paths

525: this could move outside the loop?

testing:
Probably worth testing the cli when a MUltipleDefaultRunpaths exception is raised.

This looks greats, thanks for taking on this on. I'm looking forward to seeing this land.

Brock

On 01/27/11 07:39 PM, Tim Foster wrote:
Hi all,

I've got a webrev here that allows pkgdepend users to bypass generation
of dependencies against certain files and allows them to modify the
runpath that pkgdepend produces.  I'd appreciate a code review please?

http://cr.opensolaris.org/~timf/pkgdep-python-webrev/

My plan is to get this change putback to the pkg-gate once reviewed,
then work with the ON gatekeepers to find a good time to force the
gatelings to upgrade to these pkg bits, via flag day.

I'll then push the corresponding ON changes (I've a bit more work to do
for sparc, i386 is looking good) that use this new functionality, but
will probably leave pkgdepend error-checking in its current state.

Once that's done, I'll merge and push the ON pkglint changes and enable
pkgdepend error checking at the same time.

        cheers,
                        tim


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

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

Reply via email to