On Mon, 2008-02-04 at 08:48 -0800, Dan Nicholson wrote: > Now to the review. I took a quick glance, but probably need to look again. > > 0001: Is there reason to export PM_FUNCTIONS in pm-action? I believe > everything is done in process.
Well, if we don't export PM_FUNCTIONS in pm-action and pm-powersave, we would have to export it in functions, and that seemed a bit weirdly recursive to me. Everything is done in process? The hooks, at least, are separate processes. > 0006: This could come later, but I think it would be nicer to collect > all the relocatable directories into variables at the top of the > script instead of substituting @PM_*@ throughout the script. Like: > [EMAIL PROTECTED]@ > [EMAIL PROTECTED]@ > ... > for cfg in "$PM_UTILS_SYSCONFDIR"/config.d/*[!~] ; do > > That would pave the way where you could override things at runtime for > testing or whatnot. Maybe it's not worth the trouble. It sounds like a good idea -- I did not think of collecting all the relocatable stuff at the top of functions, I just did a global search and replace in vim. The version I end up pushing upstream will have this change. > 0007: Any reason to change this from install-exec-hook to > install-data-hook? They're links to scripts, so they should be > considered part of the executables, I think. Because (oddly enough), automake thinks of scripts as data that happens to be executable, not as executable files. I suppose since you normally don't compile scripts... I ran into this issue with my first attempt at making the scripts relocatable -- the make process would attempt to rewrite the scripts before I had moved them into place, even though having the rewrite rule in install-exec-hook should have guaranteed that the scripts were in their final location. > Looks good besides those nits, though. Thanks! -- Victor Lowther Ubuntu Certified Professional _______________________________________________ Pm-utils mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/pm-utils
