Hi Wen-Yen Chuang-- On 03/04/2010 02:07 PM, Wen-Yen Chuang wrote: > The package can be found on mentors.debian.net: > - dget > http://mentors.debian.net/debian/pool/main/k/keynav/keynav_0.20100302.2713-1.dsc > > I would be glad if someone uploaded this package for me. :-) Thanks for packaging keynav -- it's looking good, and i'd say it's very close to upload-worthy as it stands. The one thing stopping me from uploading now is that the new package seems to introduce a system-wide config file, /etc/keynavrc. This isn't bad in itself, but maintaining a system config file is more work than other regular files because of accommodating/not-breaking local admin changes. And doing things like changing the location of an established configfile is a pain. So i just want to make sure you're confident that /etc/keynavrc is the right filename to use here. For example, if you can imagine a system-wide config directory in the future, you might prefer to start off with /etc/keynav/keynavrc to avoid cluttering up /etc with more files in the future. (of course this means that the system-wide config filename would be different in debian from on other systems -- that's a tradeoff you'd need to decide). Anyway, let me know what you think about this question. I haven't looked for a citation of relevant debian policy or best-practices guidelines -- if you've got links i'd love to see them. In my review, i also came up with a couple other questions that you might want to consider: * have you offered your keynav.1 manpage upstream? Jordan's pretty good about accepting patches from distributors in my experience. That way, other keynav users could take advantage of your good documentation, and it would be more likely to get attention from upstream too (i've just now subscribed to keynav-users, so i don't know if you've had this discussion with Jordan already) * debian/copyright looks machine-readable. That's great! but it doesn't seem to have a Format-Specification: line to indicate what version of the machine-readable format you're using. have a look at http://svn.debian.org/wsvn/dep/web/deps/dep5.mdwn for more details. * you're cleaning up a spurious a.out file that the build process generates. it looks to me like that file is the result of the ld test. you could probably fix that test to avoid creating the a.out in the first place, and offer it back upstream for cleanliness. want to try? * keynav.1 doesn't mention the system-wide config file location or indicate whether the user's config file overrides the system-wide one. it's looks pretty easy to tell from the code, or by experimenting, but users shouldn't have to read the code or experiment to figure that out. Let me know what you think about these issues. Based on the state of the package so far, I'd be happy to do an upload when i've heard back from you about them. Regards, --dkg
Attachment:
signature.asc
Description: OpenPGP digital signature